Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for overriding settings per request #2935

Open
wants to merge 1 commit into
base: 5.0
Choose a base branch
from

Conversation

tunniclm
Copy link
Contributor

This commit adds res.settings, an object on to which properties can be defined that will override application settings within Express internals (the values returned from app.get() are not altered, therefore any middleware/components that wants to respect the override would need to be updated to use res.settings instead of app.get()).

At present, this affects the following settings:

  • view
  • view engine
  • views
  • etag fn
  • json replacer
  • json spaces
  • jsonp callback name

The res.settings object is passed through to view rendering code by adding property _settings to the options parameter of app.render().

The entry key for the view cache was changed to prepend the views setting to ensure the cache continues to work correctly if views is overridden to a different value (eg if view 'users' was cached
from /projdir/views/a but views was overridden to point to /projdir/views/b for this request, it should not retrieve the 'users' view from the cache for /projdir/views/a).

res.settings sets its __proto__ to be the settings property of the current application so settings read from the object will default to the application settings.

As the request progresses through routing, the __proto__ will be updated to ensure values always defalt to the settings of the current application.


Fixes #2849

This commit adds `res.settings`, an object on to which
properties can be defined that will override application
settings within Express internals (the values returned
from `app.get()` are not altered, therefore any
middleware/components that wants to respect the override
would need to be updated to use `res.settings` instead
of `app.get()`).

At present, this affects the following settings:
* view
* view engine
* views
* etag fn
* json replacer
* json spaces
* jsonp callback name

The `res.settings` object is passed through to view rendering code
by adding property `_settings` to the `options` parameter of
`app.render()`.

The entry key for the view cache was changed to prepend the `views`
setting to ensure the cache continues to work correctly if `views`
is overridden to a different value (eg if view `'users'` was cached
from `/projdir/views/a` but `views` was overridden to point to
`/projdir/views/b` for this request, it should not retrieve the
`'users'` view from the cache for `/projdir/views/a`).

`res.settings` sets its `__proto__` to be the `settings` property
of the current application so settings read from the object will
default to the application settings.

As the request progresses through routing, the `__proto__` will
be updated to ensure values always defalt to the settings of the
current application.
@tunniclm
Copy link
Contributor Author

One thing I'm not sure on is the whether a "views" value of "/a" should be considered different to "/a/" or "/a/b/.." etc. The PR assumes the conservative position that they are different and each would create a distinct cached view. If Express can assume these represent hierarchical file paths for all view engines, then it might make sense to canonicalise them when calculating the cacheKey.

@tunniclm
Copy link
Contributor Author

This will need modifying to address the "special" options 'etag', 'query parser' and 'trust proxy'.
See https://github.com/expressjs/express/blob/5.0/lib/application.js#L357

@boycce
Copy link

boycce commented Feb 15, 2018

Related #3536

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants