-
-
Notifications
You must be signed in to change notification settings - Fork 381
feat: etag support #746
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
feat: etag support #746
Changes from all commits
eee8640
768d0c0
bf79de7
6a28828
203c767
7aca742
494728c
a2b418d
0129e2c
c9f954f
1bd3270
420c6fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,11 +2,35 @@ import path from 'path'; | |
|
||
import mime from 'mime-types'; | ||
|
||
import getETag from 'etag'; | ||
|
||
import getFilenameFromUrl from './utils/getFilenameFromUrl'; | ||
import handleRangeHeaders from './utils/handleRangeHeaders'; | ||
import ready from './utils/ready'; | ||
|
||
const { hasOwnProperty } = Object.prototype; | ||
|
||
export default function wrapper(context) { | ||
let etagRegistry; | ||
|
||
if (context.options.etags) { | ||
etagRegistry = new Map(); | ||
context.compiler.hooks.done.tap('webpack-dev-middleware', (stats) => { | ||
etagRegistry.clear(); | ||
|
||
try { | ||
const { assets } = stats.compilation; | ||
for (const assetId in assets) { | ||
if (hasOwnProperty.call(assets, assetId)) { | ||
const { existsAt: fsPath } = assets[assetId]; | ||
const etag = getETag(assets[assetId].source()); | ||
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. Seems this is no longer possible in Webpack 5, due to error:
@alexander-akait Do you have a recommended work around for this? 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. I ended up leveraging const { chunkHashs: assetHashes } = stats.compilation.records;
for (const assetId in assetHashes) {
if (hasOwnProp(assetHashes, assetId)) {
etagRegistry.set(assetId, JSON.stringify(assetHashes[assetId]));
}
} If there's an alternative you recommend, LMK. 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. /cc @sokra What do you think? |
||
etagRegistry.set(fsPath, etag); | ||
} | ||
} | ||
} catch (_ignoreError) {} // eslint-disable-line no-empty | ||
}); | ||
} | ||
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. No hooks here 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. We already can load assets here, please look at logic 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. I saw that there's access to the memfs instance, but is there a manifest object (eg. build stats) I can iterate over to get all the asset paths? 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. Why you need 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. Anyway if you need this, you have 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. Sounds good. I want to iterate a list of distribution files to efficiently calculate etags. If I only have the memfs instance, I'd have to traverse the entire fs, which isn't optimal. |
||
|
||
return async function middleware(req, res, next) { | ||
const acceptedMethods = context.options.methods || ['GET', 'HEAD']; | ||
// fixes #282. credit @cexoso. in certain edge situations res.locals is undefined. | ||
|
@@ -49,6 +73,21 @@ export default function wrapper(context) { | |
return; | ||
} | ||
|
||
if (etagRegistry) { | ||
const assetEtag = etagRegistry.get(filename); | ||
if (assetEtag) { | ||
const { 'if-none-match': ifNoneMatch } = req.headers; | ||
if (ifNoneMatch) { | ||
if (assetEtag === ifNoneMatch) { | ||
res.status(304).end(); | ||
return; | ||
} | ||
} else { | ||
res.set('ETag', assetEtag); | ||
} | ||
} | ||
} | ||
|
||
try { | ||
content = context.outputFileSystem.readFileSync(filename); | ||
} catch (_ignoreError) { | ||
|
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.
Seems
existsAt
doesn't exist in Webpack 5 either