-
Notifications
You must be signed in to change notification settings - Fork 1
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
Build Fails with Dynamic Routes #1
Comments
Thanks for your interest, I think this is the first non-trivial issue I've got from someone I don't know 😄. I originally decided against adding support for dynamic routes as I thought it required SSR and I don't think service workers are that beneficial in that case (since you probably can't add an offline mode). However, I just looked into fallback pages and it looks like it's worth supporting since there's no SSR to complicate things. If I'm understanding you correctly, you're using this in your top level +layout.ts? export const prerender = false;
export const ssr = false; I haven't worked on this package in a while so I've forgotten some things but I do remember that there being a set number of routes is an assumption that the code makes in quite a few places. The code currently also isn't set up to handle special routes like the fallback page. Anyway, I had a look through the code and made a list of (at least some of the) things that depend on these assumptions. They all seem to be in the worker:
And a few other things that would need to be changed:
Unfortunately I don't have much time to work on this package anymore as I've now got more obvious gaps to fill in my portfolio. I might add some proper tests though at some point, depending on what happens with my career. However, if you're up for trying to add this, I'd be very happy to be a maintainer and I might be able to lend a hand with a few things. I think I've laid some good groundwork here so it should be significantly easier than making something from scratch. But if you're just trying to get something working quickly, you might have better luck using Workbox with Vite PWA (although I kind of made this package out of frustration trying to use them 😅). Any thoughts? |
I also just updated the TODO.md file. That top bug fix might as well go into the update as it should only involve changing a couple of lines. |
Just thought of one other problem: the worker currently tries the network by default if it can't find the requested resource, which is a behaviour you might want to rely on sometimes. The problem is that's not compatible with fallback routes if the device is offline. I thought of 2 possible fixes:
And I suppose a 3rd would be to try the network with a timeout and a fallback, but that would unnecessarily slow down the first load. Edit: Also, some similar changes might have to be made for the other request modes, but since they aren't used for navigations (except with the search param if someone wants to do that for some reason), they might not need changing. |
Hey, thanks for your quick and thorough reply! I didn't realize how much of this adapter relied on having
Reading all the considerations you laid out that must be met in order to support fallback pages, I also won't have time to work on this despite my initial offer :( For now, we aren't using this adapter anymore to unblock ourselves. But I do wish this project the best - certainly the groundwork is there! |
Ah ok. Well, thanks for bringing this to my attention. |
We are looking to use this adapter to build our PWA, but we have dynamic routes that we'd traditionally need to specify a fallback page for. But we have no way to specify it because Adapter-Versioned-Worker does not expose those to us.
It would be helpful if this adapter's config includes a bit where we can specify adapter-static settings, which would then be consumed:
Please consider, and I'm happy to create a PR if needed. Thank you!
The text was updated successfully, but these errors were encountered: