express
express copied to clipboard
expressjs
Change the interface for setting application settings
The interface app.set(<name with space>, <value>) feels a odd for setting values in a JavaScript framework. Also, its counterpart app.get(<name>) is a confusing twin of app.get(path, callback [, callback ...]), which is a router method.
I am proposing to implement application settings using getter-setter interfaces.
app.settings.trustProxy = true;
And get rid of app.get(<name>).
Super major change, I know, but the next major version is a good time to make these interface improvements.
Related issues
- https://github.com/expressjs/express/issues/2216
- https://github.com/expressjs/express/issues/3997
I believe there is already a PR solving this issue that is scheduled to land on 5, using a new settings module, right @wesleytodd ?
I see. That PR is https://github.com/expressjs/express/pull/3714. It still maintains the .set(<name with space>, <value>) syntax.
I am wondering if there are any advantages of providing functions to set and get values over using getter-setter's.
app.settings.set('trust proxy', true) vs app.settings.trustProxy = true
app.settings.get('trust proxy') vs app.settings.trustProxy
Or is it just an artefact from the "pre-Object.defineProperty" era?
~~Yeah I have this PR: #3714~~ Lol, I did not read well. I see that you already got the PR link... :P
As for getters/setters vs functions, I don't have a strong objection to supporting both, it would be pretty simple using a Proxy. But, that would only land in 5 which drops support for versions w/o Proxy then we should be fine. The version targeted for 4.x cannot really support that.
When I get some time I really want to move that package over to pillarjs. I just need @dougwilson to give me the proper privileges on that org. Once that is there we can work on cleaning up the PR and making sure that docs are updated and such.
The version targeted for 4.x cannot really support that.
Not looking for 4.x at all. 5 is what I have on mind.
Cool, well I am happy to move that repo over so we can get the new interface added. Just need that permission on pillar 👍