-
Notifications
You must be signed in to change notification settings - Fork 81
Abstract prerendering #52
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
- Start Date: 2021-05-29 | ||
- RFC PR: | ||
- Svelte Issue: | ||
|
||
# Abstract pre-render | ||
|
||
## Summary | ||
|
||
Adding support for prerendered HTML files which look for their page store data at client-side runtime (what I'd call "abstract"), instead of using the snapshot of such data as it was at build time (what I'd call "concrete"). | ||
|
||
## Motivation | ||
|
||
This will allow us to pre-render every page, even the ones that do something with the request-specific page store data in `onMount`. It is possible to do this today with no changes to SvelteKit itself, if you have a web server that can read and transform (interpolating values) into the html artifacts for each request. But doing that is messy. | ||
|
||
There's a lot more detail about motivation in the initial comment of https://github.com/sveltejs/kit/issues/1533. | ||
|
||
## Detailed design | ||
|
||
### Technical Background | ||
|
||
Right now, a pre-rendered page has code like this to hydrate (this is for a parameterized route): | ||
|
||
page: { | ||
host: location.host, // TODO this is redundant | ||
path: "/my/page/123", | ||
query: new URLSearchParams("foo=bar"), | ||
params: {"id": "123"} | ||
} | ||
|
||
Aside from the "host", those are all the concrete, specific values as they were at build time, baked into the page as literals. But one might want to use the prerendered HTML artifact in an abstract way, for variable and wildcard request parameters, in which case you'd want something more like this: | ||
|
||
page: { | ||
host: location.host, | ||
path: location.pathname, | ||
query: new URLSearchParams(location.search), | ||
params: parsePathParams(location.pathname, /^\/my\/page\/(\w)+$/, ["id"]), | ||
} | ||
|
||
Where `parsePathParams` is a simple function that gives you an object of path params. | ||
|
||
### Implementation | ||
|
||
The current state of the code is here: | ||
|
||
https://github.com/sveltejs/kit/blob/a66affec4168fae053fb6349805b60028306abcf/packages/kit/src/runtime/server/page/render.js#L136 | ||
|
||
The changes would be: | ||
|
||
* For host, remove the TODO comment and the ternary, and just always use `location.host` | ||
* For path, use `location.pathname` | ||
* For query, use `location.search` | ||
* Params is the hardest one, we'd need to insert the route regex and the positional list of parameter names into a simple `parsePathParams` function, possibly just inlined as an arrow function | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd proposed moving the parsing from built time to the runtime in #36 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, hadn't seen that before, let me look. |
||
* Update the docs where appropriate | ||
|
||
Note that there is an in progress PR for the query part in https://github.com/sveltejs/kit/pull/1511 but in my view it would be better to address these all together in one PR. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is a bit outdated - the PR was closed |
||
|
||
If we implemented this RFC, it fixes https://github.com/sveltejs/kit/issues/669 and https://github.com/sveltejs/kit/issues/1397, and partially fixes https://github.com/sveltejs/kit/issues/1561 and https://github.com/sveltejs/kit/issues/1533. This RFC is only about the page store hydration aspect but "Abstract Pre-rendering" has some other facets to it, discussed in https://github.com/sveltejs/kit/issues/1533, including the question of what values you provide for parameterized route params when adding paths to `kit.prerender.pages`. | ||
|
||
The other big open question is -- do we need a switch between "concrete" (current behavior) and "abstract" (proposed behavior) mode, perhaps in `kit.prerender`, or would be fine to just always do abstract. | ||
|
||
## How we teach this | ||
|
||
I don't know if "abstract" and "concrete" are the best terminology, could also be called "deferred evaluation", or "dynamic page store vs fixed page store," etc. | ||
|
||
Also there was some confusion about the term "parameterized route" before, just to clarify, I'm using "parameterized" to mean freely variable and unknown at build time. | ||
|
||
Acceptance would entail some update to the guides, where it suggests what kind of pages are and are not suitable for pre-rendering. If we had abstract pre-rendering, _any_ page would be suitable for pre-rendering as long as it puts all its request-specific logic in `onMount`. See https://github.com/sveltejs/kit/issues/1397 | ||
|
||
## Drawbacks | ||
|
||
I don't see any drawbacks. | ||
|
||
## Alternatives | ||
|
||
One alterative, or maybe you'd call it a workaround, is to add a layer of server-side runtime interpolation that fills in the page store for each request. This involves some ugly regex slice and dice though. I have an example of this approach, in Python, in https://github.com/johnnysprinkles/static-all-the-things/blob/22e3e98149876364d8a4317e9557f727e2287021/web/manifest_load.py#L26 | ||
|
||
Another alternative would be to just not use the page store and directly inspect the `window.location` object. This would work for the query parameters but for route parameters it would need a duplicate of the route parsing logic, including the names of the parameters, and seems like a pretty untidy solution. | ||
|
||
## Unresolved questions | ||
|
||
Just the two questions mentioned above: | ||
|
||
* It's unclear what you provide for parameterized path params when pre-rendering an abstract page. It could be anything, an "x", and underscore, the only reason it matters is it determines where the html artifact lives in the build folder, and the web server will have to find the file. This question is probably out of scope for this RFC, but wanted to mention it as a related question. | ||
* Do we need a switch between abstract and concrete mode, or can we always do abstract? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't have to use the page store. You can just use
location
on the client-side. I don't think we'd want the page store on the client to be different from the page store on the serverThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, for query strings you could circumvent the page store and look at location.search, but if you're using ssr and client routing, doesn't that make a weird situation where, on initial mount you have to use location but on subsequent pages you can use the page store? Seems more consistent if they can just always use the page store.
Also, for path params, just using location wouldn't be an option because the client doesn't have the pattern or param names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The query string and host aren't available at prerender time. You have to get them from the
location
. It's better to make this explicitThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine, it's just that pesky path params then. I wonder if we could monkey patch a "params" property onto the
window.location
object. I mean of course we could, I wonder if you think we should? That would allow "location" to give a complete, dynamic view of everything the page store would normally contain. It's kind of abusing the Location builtin, and people who use Typescript might have trouble accessing a property that's not supposed to be on Location, but aside from that seems pretty elegant to me.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though, I guess this wouldn't work for people using client routing and static ssr. They'd have to look at
location.params
on the first page, and$page.store.params
on subsequent pages.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding #36, maybe that's the answer. If you need to know the path params from
onMount
in a static pre-rendered page (at client runtime), instead of page store or location, maybe it'srouterInstance.current().params
or similar. Just want to make sure this all works even for people not using client-side transitions, e.g.router: false
in the Svelte config.