Prioritize the module loader script with link headers #203
Conversation
tests/tailor.js
Outdated
| ); | ||
| }) | ||
| .then(done, done); | ||
| }); | ||
|
|
||
| it('"shouldn\'t send preloading headers if primary fragment doesn\'t exist"', done => { |
There was a problem hiding this comment.
| @@ -249,7 +249,6 @@ describe('Tailor', () => { | |||
| .then(response => { | |||
| assert.equal(response.statusCode, 200); | |||
| assert.deepEqual(response.headers['set-cookie'], [cookie]); | |||
| assert.equal(response.headers.link, 'http://link'); | |||
There was a problem hiding this comment.
Disallow primary fragments from overriding link headers since primary fragments have no context of module loading and piping mechanism.
There was a problem hiding this comment.
We are also including their assets after the loader asset.
Codecov Report
@@ Coverage Diff @@
## master #203 +/- ##
==========================================
+ Coverage 98.29% 98.34% +0.04%
==========================================
Files 14 15 +1
Lines 586 603 +17
Branches 106 112 +6
==========================================
+ Hits 576 593 +17
Misses 10 10
Continue to review full report at Codecov.
|
lib/utils.js
Outdated
| host, | ||
| asAttribute, | ||
| corsCheck = false, | ||
| noPush = true // Disable HTTP/2 Push behaviour for now |
There was a problem hiding this comment.
Will be enabled in future once cache digests spec is implemented in browsers.
There was a problem hiding this comment.
Then maybe replace the for now with until digest spec is implemented by most browsers to help future maintainers. This PR commit won't be easily findable
|
|
||
| // Loader script must be preloaded before every fragment asset | ||
| const loaderScript = getLoaderScript(amdLoaderUrl, request.headers); | ||
| loaderScript !== '' && assetsToPreload.unshift(loaderScript); |
There was a problem hiding this comment.
any reason not to use explicit if () {} here?
There was a problem hiding this comment.
To avoid the typecasting.
| amdLoaderUrl, | ||
| request.headers | ||
| ); | ||
| loaderScript !== '' && |
There was a problem hiding this comment.
any reason not to use explicit if () {} here?
lib/utils.js
Outdated
| const LinkHeader = require('http-link-header'); | ||
|
|
||
| const getCrossOrigin = (url = '', host) => { | ||
| if (host && url.indexOf(`://${host}`) < 0) { |
There was a problem hiding this comment.
url.includes(...) is a tiny bit more expressive
|
👍 |
1 similar comment
|
👍 |
Fixes preload the module loader script to avoid getting deprioritized #202
While working on this feature, figured out a Chrome bug which causes the module loader script in preload cache to be ignored.
Link to the bug - bugs.chromium.org/p/chromium/issues/detail?id=794958
Before
After
Works only in prime cache because of the chrome bug.