(perf): improve parsing of link header from fragments #248
Conversation
vigneshshanmugam
commented
Aug 12, 2018
•
vigneshshanmugam
commented
- Roll our own fine tuned parse link header implementation based on https://github.com/thlorenz/parse-link-header
- Avoid double parsing of link headers on fragment response and while preloading
- Keep fragment object shape static
Codecov Report
@@ Coverage Diff @@
## master #248 +/- ##
=========================================
Coverage ? 99.54%
=========================================
Files ? 17
Lines ? 658
Branches ? 124
=========================================
Hits ? 655
Misses ? 3
Partials ? 0
Continue to review full report at Codecov.
|
lib/utils.js
Outdated
| @@ -80,6 +68,20 @@ const getFragmentAssetsToPreload = (headers, { host } = {}) => { | |||
| return assetsToPreload; | |||
| }; | |||
|
|
|||
| function getFragmentAssetUris(refs, assetSize) { | |||
There was a problem hiding this comment.
nit: Why put this in utils, is it used anywhere else?
Are we planning to add this to the exported API?
There was a problem hiding this comment.
Not really, I can move inside fragment.js
| assert.deepStrictEqual(parseLinkHeader(linkHeader), []); | ||
| }); | ||
|
|
||
| it('filters invalid rel attributes', () => { |
There was a problem hiding this comment.
What is an invalid rel attr?
Empty? Can I use in rel="styleshit"?
There was a problem hiding this comment.
Ofcourse you can lol. its valid. But wont be loaded because of the check in getFragmentAssetUris.
There was a problem hiding this comment.
But the test if for rel="" and no rel attribute at all. do you have any better naming suggestion if its confusing?
There was a problem hiding this comment.
maybe it could be rephrased... but it is fine.
| const styleUris = []; | ||
|
|
||
| for (const ref of refs) { | ||
| if (ref.rel === 'fragment-script') { |
There was a problem hiding this comment.
Do we have tests for this filtering behaviour?
There was a problem hiding this comment.
Yes, tests/tailor.js.
|
👍 |
1 similar comment
|
👍 |
