[feat] detect if app tries to access query with prerender enabled

[benmccann]

Closes #669

[changeset-bot]

🦋 Changeset detected

Latest commit: bda1b98

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

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

[johnnysprinkles]

Sorry, why prevent them from accessing query while prerendering? You could have fixed options for query string in the pages array, like:

kit: {
  prerender: {
    pages: [
      '/dashboard?tab=1',
      '/dashboard?tab=2',
      '/dashboard?tab=3',
    ],
  },
}

i.e. nonparameterized query string options. This change breaks that pattern right?

[johnnysprinkles]

Also, do you guys not require another pair of eyes to review each change going in?

[benmccann]

You could have fixed options for query string in the pages array, like...

I don't think you could even before this change. ? is a forbidden character on some file systems and (prerender serving doesn't look for query string when deciding an asset to server). I'm not saying we'd never support it in the future, but historically we haven't and I'd much rather let users know it's not supported up front than only finding out after much debugging.

But I'm not sure whether it is something we should support. E.g. what would happen if someone requests /dashboard?tab=4? Do you return 404, do you return the non-prerendered page? Query strings in Svelte today are not considered in routing and you'd be breaking that model, which makes it unclear how it should behave

We ask for reviews on things that are a bit larger such as changes in APIs, etc. This PR was open for almost a week, so the community could comment on it and the proposed plan was mentioned months ago in the associated issue, so didn't rise to that level

[johnnysprinkles]

Got it.

Ah yes, I totally forgot about my #1825, query strings don't even work at the moment anyway. This actually makes it so they probably won't ever, which is of course fine for new websites. The only issue might be if you're porting an existing website to SvelteKit and want to match the URL form precisely, it could be a problem. Maybe any discussion can continue in #1825.