8000 Upgrade to Express 4 by daffl · Pull Request #55 · feathersjs/feathers · GitHub
[go: up one dir, main page]

Skip to content

Upgrade to Express 4#55

Merged
daffl merged 3 commits intomasterfrom
express-4
Apr 18, 2014
Merged

Upgrade to Express 4#55
daffl merged 3 commits intomasterfrom
express-4

Conversation

@daffl
Copy link
Member
@daffl daffl commented Apr 17, 2014

Express 4.0.0 got released a week ago. This pull requests implements the changes necessary for Feathers to use it and removes the default configuration of the REST provider (#53) which makes things even easier.

Closes #53, closes #54.

@daffl daffl added this to the 1.0.0 milestone Apr 17, 2014
@pspeter3
Copy link

This looks great! I was trying to do this last night but hit some issues with refactoring the tests. Thanks for doing this! In the long term though, does feathers want to move away from the configure method?

@daffl
Copy link
Member Author
daffl commented Apr 17, 2014

Is the configure method such a bad thing (it's not doing the environment specific configuration anymore)?

I was debating of renaming it but we need some nice way to load plugins I think. This:

app.configure(feathers.rest()).use('/todos', todoService);

Looks a lot nicer than

var app = feathers();

feathers.rest.call(app);
// or
feathers.rest(app);

app.use('/todos', todoService);

@pspeter3
Copy link

I was going with the feathers.rest(app); approach because I think it more explicit

@daffl
Copy link
Member Author
daffl commented Apr 17, 2014

To me in this case being explicit feels kind of clunky. I like fluid interfaces (guess I'm jQuery damaged ;) and Express already goes with it. The biggest problem is that if you do something like:

var app = feathers().use('/todos', todoService);;

feathers.rest.call(app);
// or
feathers.rest(app);

Things won't work as expected.

It's easier to say "You need to configure providers and plugins first." vs. "You need to call the provider or plugin with your application before adding other Middleware or services.".

@pspeter3
Copy link

That makes sense

daffl added a commit that referenced this pull request Apr 18, 2014
@daffl daffl merged commit 6b72915 into master Apr 18, 2014
@daffl daffl deleted the express-4 branch April 18, 2014 21:58
@pspeter3
Copy link

👍

daffl added a commit that referenced this pull request Aug 19, 2018
* Refactoring for compatibility with Feathers v3

* Finalization of hook methods

* Change getHooks back to previous version

* Prepare for 1.0 prerelease

* 1.0.0-pre.1

* Updating changelog

* Update to new plugin infrastructure

* Update .npmignore

* Remove Node 4 from Travis

* 1.0.0-pre.2

* Updating changelog

* Update the client test suite (#55)

* Update release script

* 1.0.0-pre.3

* Updating changelog
daffl pushed a commit that referenced this pull request Aug 21, 2018
daffl added a commit that referenced this pull request Aug 21, 2018
* Refactoring for compatibility with Feathers v3

* Finalization of hook methods

* Change getHooks back to previous version

* Prepare for 1.0 prerelease

* 1.0.0-pre.1

* Updating changelog

* Update to new plugin infrastructure

* Update .npmignore

* Remove Node 4 from Travis

* 1.0.0-pre.2

* Updating changelog

* Update the client test suite (#55)

* Update release script

* 1.0.0-pre.3

* Updating changelog
daffl pushed a commit that referenced this pull request Aug 21, 2018
daffl added a commit that referenced this pull request Aug 23, 2018
…cs (#55)

* chore(package): update semistandard to version 12.0.0

* Remove example and update Readme to point directly to the Feathers docs
daffl added a commit that referenced this pull request Aug 28, 2018
daffl added a commit that referenced this pull request Aug 28, 2018
* Bring back Cookie token extractor

* Increase code coverage
daffl pushed a commit that referenced this pull request Aug 29, 2018
daffl added a commit that referenced this pull request Aug 29, 2018
daffl added a commit that referenced this pull request Aug 29, 2018
* Bring back Cookie token extractor

* Increase code coverage
daffl pushed a commit that referenced this pull request Aug 29, 2018
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.

Update to Express 4.0 REST provider should not be added by default

2 participants

0