fix: add link rel=preload for exported sites #568
Conversation
| @@ -139,38 +146,49 @@ async function _export({ | |||
| clearTimeout(the_timeout); // prevent it hanging at the end | |||
|
|
|||
| let type = r.headers.get('Content-Type'); | |||
|
|
|||
| let body = await r.text(); | |||
|
|
|||
| const range = ~~(r.status / 100); | |||
|
|
|||
| if (range === 2) { | |||
There was a problem hiding this comment.
Hiding whitespace will make the diff below a lot easier to read.
| @@ -0,0 +1,9 @@ | |||
| import * as sapper from '@sapper/app'; | |||
There was a problem hiding this comment.
I copy-pasta'd the test/apps/export project into test/apps/export-webpack, changing only rollup.config.js by replacing it with webpack.config.js.
test/apps/export-webpack/test.ts
Outdated
|
|
||
| it('injects <link rel=preload> tags', () => { | ||
| const index = fs.readFileSync(`${__dirname}/__sapper__/export/index.html`, 'utf8'); | ||
| assert.ok(/rel=preload/, index); |
There was a problem hiding this comment.
There is probably more I could do to test this, but I wasn't sure if you wanted me to include an entire HTML parsing library or something. (Cheerio?)
|
|
||
| it('injects <link rel=preload> tags', () => { | ||
| const index = fs.readFileSync(`${__dirname}/__sapper__/export/index.html`, 'utf8'); | ||
| assert.ok(/rel=preload/.test(index)); |
There was a problem hiding this comment.
There is probably more I could do to test this, but I wasn't sure if you wanted me to include an entire HTML parsing library or something. (Cheerio?)
There was a problem hiding this comment.
nah, that'd be overkill — this is what I'd have done as well 👍
| @@ -0,0 +1,73 @@ | |||
| const webpack = require('webpack'); | |||
There was a problem hiding this comment.
I copy-pasted this config file over from the only other webpack text project, with-sourcemaps-webpack.
|
|
||
| it('injects <link rel=preload> tags', () => { | ||
| const index = fs.readFileSync(`${__dirname}/__sapper__/export/index.html`, 'utf8'); | ||
| assert.ok(/rel=preload/.test(index)); |
There was a problem hiding this comment.
nah, that'd be overkill — this is what I'd have done as well 👍
|
Fixes #402. Thanks! |
|
@nolanlawson - this appears to not be working anymore... Opened #919 around this. |
I think
sapper exportshould try to preserve thelink rel=preloadheader as much as possible, which in this case means embedding it into the HTML.Also I've found that preload makes a big enough difference in load time that it's worth it. (Just tested on Pinafore using simulated slow 3G, and it's a difference of 8.7s for the load event versus 59.9s. 😬)
I added a test using Webpack because it looks like none of the Rollup tests generate preload link headers.