-
-
Notifications
You must be signed in to change notification settings - Fork 2k
docs(express.static): add accept-ranges and cache-control options to express.static #2089
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
Conversation
✅ Deploy Preview for expressjscom-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🚦 Lighthouse Results (Mobile & Desktop)
|
efekrskl
left a comment
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.
Looks good! Could you link this PR to the related issue (e.g "Fixes #????") so it closes automatically when merged? (see)
| | `maxAge` | Set the max-age property of the Cache-Control header in milliseconds or a string in [ms format](https://www.npmjs.org/package/ms). | Number | 0 | | ||
| | `redirect` | Redirect to trailing "/" when the pathname is a directory. | Boolean | `true` | | ||
| | `setHeaders` | Function for setting HTTP headers to serve with the file. <br/><br/>See [setHeaders](#setHeaders) below. | Function | | | ||
| | `acceptRanges` | Enable or disable accepting ranged requests. Disabling this will not send the Accept-Ranges header and will ignore the contents of the Range request header. <br/><br/>See [acceptRanges](#acceptRanges) below. | Boolean | true | |
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.
Could be better to put keyword inside backticks e.g Accept-Ranges
| <h5 id='acceptRanges'>acceptRanges</h5> | ||
|
|
||
| Enable or disable accepting ranged requests, defaults to true. | ||
| Disabling this will not send `Accept-Ranges` and ignore the contents | ||
| of the `Range` request header. | ||
|
|
||
| <h5 id='cacheControl'>cacheControl</h5> | ||
|
|
||
| Enable or disable setting `Cache-Control` response header, defaults to | ||
| true. Disabling this will ignore the `immutable` and `maxAge` options. | ||
|
|
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 think this section adds much, as it is the exact same description we have above
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.
yep done
|
Maybe you could put the It looks like you accidentally pushed some Gemfile changes. Other than that, looks good! |
|
i removed the changes in the Gemfile.PR description i already have the fixes and the isssue number. Can you check it and let me know if everything is good |
|
@efekrskl , Can review this and let me know of there any changes? |
|
Could you please restore your original PR title, and put the I think the PR looks good, but we'll need a team member who can merge for the final call. Hope that makes sense :) |
bjohansebas
left a comment
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.
LGTM
|
thanks @Akash-2006 |
Added missisng accept ranges and cachecontrol in _includes/api/en/4x/express.static.md and _includes/api/en/5x/express.static.md