-
Notifications
You must be signed in to change notification settings - Fork 0
feat(cachePolicy): [INFRA-1290] Add CDN caching for fx proxy recommendations #48
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,10 +2,6 @@ import { NextFunction, Request, Response } from 'express'; | |
|
|
||
| import ConfigFile from '../../config'; | ||
|
|
||
| // eslint / prettier config cannot cope with functions that return | ||
| // functions like this. Thrashes between 2 states. | ||
| // prettier-ignore | ||
|
|
||
| /** | ||
| * Sets the value of the 'Cache-Control' response header for all downstream | ||
| * express handlers. | ||
|
|
@@ -18,13 +14,16 @@ import ConfigFile from '../../config'; | |
| * | ||
| * @param cachePolicy string value to set CacheControl response header | ||
| */ | ||
| const CacheControlHandler = | ||
| (cachePolicy: string, config: typeof ConfigFile) => | ||
| (req: Request, res: Response, next: NextFunction): void => { | ||
| if (config.app.environment !== 'development') { | ||
| res.set('Cache-control', cachePolicy); | ||
| } | ||
| return next(); | ||
| }; | ||
| const CacheControlHandler = ( | ||
| cachePolicy: string, | ||
| config: typeof ConfigFile | ||
| ) => { | ||
| return (req: Request, res: Response, next: NextFunction): void => { | ||
| if (config.app.environment !== 'development') { | ||
| res.set('Cache-control', cachePolicy); | ||
| } | ||
| return next(); | ||
| }; | ||
| }; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. very very tiny refactor to get rid of the eslint / prettier errors. Works as expected but I wasn't sure the original intent behind not having a Can revert
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is equivalent. It's just another syntax that can avoid nesting if your lint rules allow it. Typically when I use higher order functions I would format it like this: The second function is interpreted as single expression and implicitly returns. It's nice and concise, but the eslint / prettier rules don't really like this at all though. Docs |
||
|
|
||
| export default CacheControlHandler; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have much knowledge around Web repo but I'm assuming this is where web repo sets the cache time limit for recommendations:
https://github.com/Pocket/Web/blob/f50a32402077629b61c55930941814c442a6ff19/public_html/v3/firefox/global-recs.php#L33
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks correct to me.