Skip to content

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

Closed
wants to merge 1 commit into from
Closed

Abstract prerendering #52

wants to merge 1 commit into from

Conversation

johnnysprinkles
Copy link

* 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
* 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is a bit outdated - the PR was closed

@johnnysprinkles
Copy link
Author

Here's how this would look, not submitting for PR or anything but just wanted to demonstrate the changes we'd need to make. Not very deep changes:

https://github.com/johnnysprinkles/kit/commit/963f0471184c245799744de7befa4ac85e82c56d

I think something like this would ultimately be a prerequisite for doing i18n as well. We'd ultimately need a prerenderer that can render each route multiple times based on an arbitrary number of dimensions, and presumably we'd pass information about which value for each dimension via query string. That communication is going to be different at runtime and at build time, for example the current location at runtime might just be geolocated and not use the query string at all. I'll post more thoughts in the i18n mega thread.

@johnnysprinkles
Copy link
Author

I really think we should reconsider the discussion in sveltejs/kit#1511 that got shut down abruptly by a @Rich-Harris comment (sveltejs/kit#1511 (comment)). cc @pz-mxu

It's quite useful to be able to pre-render pages for which params aren't known at build time, and I certainly plan on doing just that.

@johnnysprinkles
Copy link
Author

Another possible approach would be do not do any of this, and let client-only code that needs to know query or route params simply not use the page store and get it directly from location. The only issue there is the client doesn't know the regex pattern or positional list of param names. We could just put that somewhere where the client code could get it.

@johnnysprinkles
Copy link
Author

johnnysprinkles commented Jul 4, 2021

Oh what am I thinking? runtime/client/renderer.js already knows how to parse the URL according to the current route. All I'd need to do is make it re-run its page store generation logic at the appropriate time, to overwrite the wildcard one that was captured in the page store from build time. Let me investigate this path.

@johnnysprinkles
Copy link
Author

johnnysprinkles commented Jul 4, 2021

So just to summarize, I have two approaches to demonstrate and a couple others to consider:

  1. Redefine what the page store means, when pre-rendering is involved. Currently it means "the values as they were at pre-render time." This proposes that they become "the values as they currently are for the current request at runtime." Might be rather invasive to redefine things like this though. https://github.com/johnnysprinkles/kit/commit/963f0471184c245799744de7befa4ac85e82c56d
  2. Make the primitives of the SSRRoute data available in the client somehow, maybe via singletons and $app/navigation, so they can parse the window.location themselves to get path params. Fewer changes in server/page/render.js https://github.com/johnnysprinkles/kit/commit/e83bd2151d55a0561c260b073358d4490067bf09
  3. Maybe just add a simple hack that navigates to the current page, to get the page store to update? The downside of this is now we depend on the client router, which I wasn't planning to use. This seems like a non-starter.
  4. Add two more fields into the page store, "routePattern" and "routeParamNames". For people who use client-side routing, this should update as you navigate around, even though I can only see it being useful on the first page. Now you can look at the route param values and if any seem dynamic (have colons, square brackets, or whatever convention you want to use), you can parse the path params from the URL yourself. This seems like the most correct way to go.

* 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
Copy link
Member

Choose a reason for hiding this comment

The 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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, hadn't seen that before, let me look.


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: {
Copy link
Member

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 server

Copy link
Author

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.

Copy link
Member

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 explicit

Copy link
Author

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.

Copy link
Author

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.

Copy link
Author

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's routerInstance.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.

@johnnysprinkles
Copy link
Author

Cleaning up a little. I was thinking of SvelteKit as a comprehensive solution that fits into your overall software architecture, when in fact it's more humble than that. Mostly just a platform to start with Svelte.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants