Settings functionality moved out of core

[wesleytodd]

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:

1. I opted to move it all under .settings, mainly to resolves the weird behavior from app.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.
2. If we are already breaking the api, should we also add other changes, like removing the enabled/disabled shorthands to minimize the api surface.
3. Do people like the "setters" type functionality? It is used to keep the features like setting etag but it actually setting etag fn. Not sure, but it seems to me we could get rid of the whole thing but just using etag directly.

EDIT: standardjs compliance pending :)
**Dont feel like doing all that busy work tonight if the whole PR might get shut down...

[wesleytodd]

Is there something I can do on this PR to move it forward?

[dougwilson]

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).

[wesleytodd]

Awesome!! I will go through and do a rebase/cleanup pass asap.

[dougwilson]

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).

[wesleytodd]

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.

[dougwilson]

Nice.

[wesleytodd]

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 :)

[wesleytodd]

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.

[abrahamcuenca]

What is the status of this PR?

[thernstig]

@wesleytodd did this die off? Looking at #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).

[wesleytodd]

I am not sure this one can land without addressing the one which targeted v4 #3714. I think we were unclear what we wanted in that thread, so I might need to think on it for a bit and get feedback from the other folks on @expressjs/express-tc if we want to try and land this one with or without the v4 deprecation.

[UlisesGascon]

I agree that we might want to discuss this in more detail.

Overall, I am in favor of this approach as app.settings.* is clearer. I think we need to include the deprecation in v4. I am happy to help with the re-work needed for this to land in v5 and to add the deprecations in v4 if we agree on it.

[wesleytodd]

We discussed this on the call today and have decided that we would not land this. In the future the plan is to revamp the settings api to make it more suitable to the needs of modern express apps. So that we don't break folks twice for the same api we decided this one can be closed.