Issues with Sapper in pinafore.social

[nolanlawson]

I promised Rich that I would file some bugs/feature requests after launching https://pinafore.social, but unfortunately I haven't had a lot of time to make reductions/etc., so I'm opening a mega-issue to describe problems I ran into; feel free to ignore or close as you see fit. 😊

First off I should say I'm using a fork of Sapper from early January, so I'm not in a rush to have these issues fixed, since everything Works For Me™. But I thought it'd be worthwhile to at least provide the feedback.

Missing features:

1. Ability to specify custom Cache-Control values for different endpoints. I wanted to tweak these values, e.g. to change the cache duration and to add Cache-Control:immutable to immutable Webpack resources.
2. Compatibility with mini-css-extract-plugin; these add .css chunks for every .js chunk, so I needed to update the preload headers and inlined styles accordingly. See better JS size and async CSS with mini-css-extract-plugin nolanlawson/pinafore#145
3. Option to disable route preloading; I removed this because it didn't seem necessary to me and was loading a lot of unused JS (nolanlawson@f6b505d)

Bugs:

1. I have some routes like /foo/bar.html and /foo/[id].html. In the old version of Sapper, /foo/bar would use the bar.html component, whereas in the new version of Sapper, it uses the /foo/[id].html component. It feels like the non-wildcard should match first.
2. In Sapper 0.9.6 some of the Webpack chunks fail to load, which breaks routing on some pages and was a showstopper bug for me. Steps to repro can be run using this Pinafore branch where I was attempting to upgrade Sapper:

git checkout --branch upgrade-sapper https://github.com/nolanlawson/pinafore.git
cd pinafore
npm run dev

1. Navigate to localhost:4002 in any browser
2. Click "add an instance"
3. Enter instance (e.g. mastodon.social)
4. Log in, authorize

On redirect, you'll see this error:

Error: loading chunk 0 failed
Uncaught (in promise) Error: Loading chunk

I suspect the issue has something to do with Webpack 4 and the new splitChunks functionality, because the chunk that fails to load seems to be a "common" chunk (i.e. it has lots~of~tildes~like~this indicating it's shared by multiple chunks).

Again, sorry for the mega-issue and for not providing a PR to help out. The work you've put in to Svelte and Sapper are incredible, and the code is readable and comprehensible enough that I'm happy using my January fork for now. Hopefully I'll be able to complete the upgrade soon and offer some PRs to help make it happen.

[Rich-Harris]

Thank you! I'll be honest, I was braced for a much longer list than this, so colour me pleasantly surprised.

Route preloading is disabled by default in the most recent version — there's a prefetchRoutes function you can use to do it manually instead (docs). Apart from that, Sapper has been slightly neglected recently as the big Svelte v2 release has been all-consuming, but yep, these are all things we need to fix. I'm surprised by the route priority bug, I thought those were all squashed now 😬

And congrats on Pinafore! I haven't been able to get into my mastodon.social account (it won't accept my 2FA code) so haven't really been able to give it a spin wait I just remembered I have a toot.cafe account! Here we go...

[nolanlawson]

Thanks for the feedback! The route priority bug may indeed be fixed. I tested this about a month ago and wrote a workaround in my upgrade-sapper branch.

The other issue (the Webpack 4 one) is definitely still active, though. I just tested in Sapper 0.9.6 to make sure.

[nolanlawson]

Another issue I ran into recently is with the service worker timestamp.

If the timestamp used in service-worker.js is generated at build time, and if you're using something like now which scales, then users may end up getting constant service worker updates because every time they hit a different node they get a different timestamp. This manifested for us as an "app update available" message that appeared too frequently: nolanlawson/pinafore#80

My solution was to use process.env.SAPPER_TIMESTAMP || Date.now() when generating the timestamp, and then pass in the SAPPER_TIMESTAMP via now -e SAPPER_TIMESTAMP=$(date +%s%3N). This guaranteed that every node used the same timestamp.

An alternative may be to generate a unique hash based on the actual contents of the service worker (which would thus be unchanged across builds). But I opted not to do that, because I have other logic in place to sort the caches by timestamp and keep the last 2, to work around issues where a service worker may delete a webpack asset before the user reloads, which would then break when the user tries to load an async chunk.

[Rich-Harris]

I think we can close this issue now as everything can either be considered resolved or has a dedicated issue:

Ability to specify custom Cache-Control values for different endpoints. I wanted to tweak these values, e.g. to change the cache duration and to add Cache-Control:immutable to immutable Webpack resources.

I've opened #415 to track this

Compatibility with mini-css-extract-plugin; these add .css chunks for every .js chunk, so I needed to update the preload headers and inlined styles accordingly. See nolanlawson/pinafore#145

For Rollup apps, CSS is now extracted automatically, and there's an issue regarding preloading (#408). For webpack there's #407.

Option to disable route preloading; I removed this because it didn't seem necessary to me and was loading a lot of unused JS (nolanlawson/sapper@f6b505d)

Route preloading is disabled by default, as noted above

I have some routes like /foo/bar.html and /foo/[id].html. In the old version of Sapper, /foo/bar would use the bar.html component, whereas in the new version of Sapper, it uses the /foo/[id].html component. It feels like the non-wildcard should match first.

This has been robustified a lot since then (completely different mechanism) so I'm fairly certain this is fixed

In Sapper 0.9.6 some of the Webpack chunks fail to load, which breaks routing on some pages and was a showstopper bug for me.

I'm not aware of any similar reports so I'm going to optimistically assume that this was a short-lived bug (bringing the repro up to date with current versions is going to be a little tricky)

Another issue I ran into recently is with the service worker timestamp.

Interesting... I don't think we need a separate issue for this, as it can be solved in userland as you demonstrated. I'd be wary of the hash-based solution since it could result in zombie caches (if the service worker itself was unchanged, but assets/generated content had changed).

Thanks!