-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Don't pass params from the server #4935
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
Conversation
🦋 Changeset detectedLatest commit: dde30cd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Note that I started documenting various SvelteKit patterns, that I plan to reference in feature requests and PRs going forward. This particular PR would be in service of Pattern IV. See https://royalbarrel.com/notes/3 |
Just rebased, and also I verified that this change covers both the page store and the |
OK, this is rebased today and ready for review. Let me know if there's a better way to parse the URL params rather than running it through |
Apologies for leaving this PR out in the sun for so long — sometimes when there's enough activity on the PR page things like this sort of get trudged into the ground until you forget to pick them up. The original reason for passing There's a variety of embeds available; the one that gets selected is based on There are definitely more elegant ways that problem could be solved, though they'd involve either bypassing SvelteKit or adding some sort of embedded output that renders self-contained-HTML-plus-hydration code but omits the SvelteKit runtime. That said, the current approach does have some benefit: So I'd love to better understand what the motivation for this is. If you're using |
This is really interesting, I've never thought about rendering "embeds" just whole pages, which presumably live among the same URL structure at runtime as they do at build time. I can tell you about my use case: We have several routes, some of which are parameterized, for example a parameterized one would be like:
Where the ID is a numeric value. This won't prerender by default so we explicitly prerender it with a placeholder value, i.e. an entry like this in the svelte config prerender entries:
Which means pass the string "id" as the ID. Then we do all the value specific data fetching clientside only (either in So now in our build folder we have a /song/id.html file (or is it /song/id/index.html, not sure, recalling this from memory). Then we just rewrite all And this all works great except that getting the "id" value has to be done manually. We have code like this that I'm again recalling from memory:
Not too bad just adds a place we have to update if the paths ever change. |
* [breaking] add embedded option, turned off by default Closes #7940 Adds a new option which defaults to false. If true, events related to navigation etc are listened to on the target element rather the html root * fix test * support #4935 * tests * docs * filter start options Co-authored-by: Rich Harris <[email protected]>
I'd argue that we shouldn't pass the path params from server to client, for the same reason we're no longer passing the URL -- the real version on the client is the authoritative version.
This to me is just a logical extension of #3942.
This change would fix #1533, and obviates #1871.
The motivation for this is, in Big Tech, you often find yourself in a Java monoculture. No problem though, adapter-static to the rescue. But the issue is, in general, every website will have parameterized paths and it's a little ugly using adapter-static with parameterized routes. You can do it but you have the regex-parse the URL yourself instead of relying on the pagestore provided params. This fixes that issue.
As with my previous PR from a few days ago, not super optimistic about this going in, but useful to have for reference in case anyone wants to patch it or make a fork.
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. All changesets should bepatch
until SvelteKit 1.0