express
express copied to clipboard
Settings functionality moved out of core
This is based on what was discussed at the last TC meeting for a good "first step" in abstracting out parts of core than can be shared. The changes here are rather far reaching, but they basically mean the following:
app.set()
app.get()
app.enable()
app.disable()
app.enabled()
app.disabled()
Become:
app.settings.set()
app.settings.get()
app.settings.enable()
app.settings.disable()
app.settings.enabled()
app.settings.disabled()
I think there are a bunch of points for discussion here:
- I opted to move it all under
.settings
, mainly to resolves the weird behavior fromapp.get
being multi purpose. But it could also be done so that the methods remain the same, either via mixing them in, or explicitly calling into.settings
as getter methods. - If we are already breaking the api, should we also add other changes, like removing the
enabled
/disabled
shorthands to minimize the api surface. - Do people like the "setters" type functionality? It is used to keep the features like setting
etag
but it actually settingetag fn
. Not sure, but it seems to me we could get rid of the whole thing but just usingetag
directly.
EDIT: standardjs compliance pending :) **Dont feel like doing all that busy work tonight if the whole PR might get shut down...
Is there something I can do on this PR to move it forward?
Love it. There hasn't been any objections or anything, and finally gets rid of those annoying overloads like .get(setting)
. I plan to land this right in the next 5.x release coming here (which will also include the new Promise-supporting router).
Awesome!! I will go through and do a rebase/cleanup pass asap.
Sweet, that will help a lot 👍 I had noticed two things, mainly, from looking through it at a high level: (1) cap the dependency to ~
instead of ^
(or pin) and (2) I was curious to see if there would be some way to back port this into 4.x such that there could be a deprecation cycle on the removed APIs (but I don't know if that's possible -- and it's not a requirement to land something breaking into 5.x, of course).
Yeah I can cap that for sure. For the back porting, I think we can do it, just need to call into the new stuff and log the deprecation in the old. I will take a look at that as well.
Nice.
Ok, so the changes in 4.x
are so different that it will actually be better for me to make two separate PR's. Other than the examples, basically every line changed is a conflict. I will open up a new PR with the 4.x compatible changes and then see what is required to the 5.0 support. But unfortunately it has eaten up all of my time today to be sure that is the best way to move forward. Maybe next weekend :)
Ok, #3714 is open. It ports all these changes to 4.0 and provides deprecation warnings. Once we figure out the points I brought up over there I will finalize the changes in this PR for the 5.0 branch.
What is the status of this PR?
@wesleytodd did this die off? Looking at https://github.com/expressjs/express/pull/2237 it seems sensitive to ask about 5.0 considering it is locked. Considering that PR was created on 14 Jul 2014 I think it is a fair question (although I do know these topics are sensitive to maintainers, but 7 years is a long time).