Skip to content

Deprecate built-in CORS middleware [2.x]#357

Merged
bajtos merged 1 commit into
2.xfrom
feature/deprecate-cors-2x
Sep 22, 2016
Merged

Deprecate built-in CORS middleware [2.x]#357
bajtos merged 1 commit into
2.xfrom
feature/deprecate-cors-2x

Conversation

@bajtos

@bajtos bajtos commented Sep 21, 2016

Copy link
Copy Markdown
Member

Push the responsibility of enabling/configuring CORS back to the application developer.

Back-port #352

@richardpringle or @deepakrkris please review

Push the responsibility of enabling/configuring CORS back to the
application developer.
Comment thread test/rest.browser.test.js
// setup
beforeEach(function() {
objects = RemoteObjects.create();
objects = RemoteObjects.create({cors: false});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to set CORS in all the tests? Should these tests not use the default settings?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default setting is to enable the built-in CORS middleware with { origin: true, credentials: true }, which triggers deprecation warnings. That was my intention, to poke 2.x users to rework the CORS settings in their application soon.

This applies to our tests too. If I didn't add {cors: false} to all the tests, then npm test prints a lot of deprecation warnings.

Considering the amount of changes in strongloop/loopback-component-explorer#178 too, perhaps it was not a good idea to add this deprecation message?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! I didn't think about those messages crowding the test results!

I think you should leave them in.

@richardpringle

Copy link
Copy Markdown
Contributor

The deprecation messages look fine

@richardpringle

Copy link
Copy Markdown
Contributor

approved!

@bajtos bajtos closed this Sep 22, 2016
@bajtos bajtos reopened this Sep 22, 2016
@bajtos bajtos added #review and removed #review labels Sep 22, 2016
@bajtos
bajtos merged commit d3c6b9a into 2.x Sep 22, 2016
@bajtos
bajtos deleted the feature/deprecate-cors-2x branch September 22, 2016 12:22
@bajtos bajtos removed the #review label Sep 22, 2016
@bajtos bajtos changed the title Deprecate built-in CORS middleware Deprecate built-in CORS middleware [2.x] Sep 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants