kit icon indicating copy to clipboard operation
kit copied to clipboard

Handle `entries` when `+page.ts` are `+server.ts` in the same directory.

Open hyunbinseo opened this issue 2 years ago • 9 comments

Describe the bug

In the following setup,

tree src/routes 

src/routes
└── [slug]
    ├── +page.svelte
    ├── +page.ts
    └── +server.ts
// src/routes/[slug]/+page.ts
export const prerender = true;
export const entries = () => [{ slug: '1' }];

the pre-rendering and production router are broken.

Browser Dev Preview
/1 +page.svelte rendered 404 Not Found
npm run build && tree .svelte-kit/output/prerendered

.svelte-kit/output/prerendered  [error opening dir]

Reproduction

Reference the StackBlitz project.

Currently, removing the +server.ts file seems to be the only solution.

An empty +server.ts does not work either.

Logs

No response

System Info

├─ @sveltejs/[email protected]
├─ @sveltejs/[email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]

Severity

serious, but I can work around it

Additional Information

If the +server.ts exports entries,

// src/routes/[slug]/+server.ts

import { json } from '@sveltejs/kit';
import type { EntryGenerator, RequestHandler } from './$types';

// Uncomment these two lines in the StackBlitz project.
export const prerender = true;
export const entries = (() => [{ slug: '2' }]) satisfies EntryGenerator;

export const GET = (({ params }) => json(params)) satisfies RequestHandler;

only this gets pre-rendered.

Browser Dev Preview
/1 +page.svelte rendered 404 Not Found
/2 +page.svelte rendered {"slug":"2"}
npm run build && tree .svelte-kit/output/prerendered

.svelte-kit/output/prerendered
└── pages
    └── 2

hyunbinseo avatar May 15 '23 17:05 hyunbinseo

Okay, this is weird. I can clearly see that the prerendered content folder isn't being created in the StackBlitz repository, but whenever I try to reproduce this locally, it works just fine. Can you do me a favor and try to reproduce this with a GitHub repository and share the link?

As for the second part, this is expected -- SvelteKit checks +server.ts, +page.server.ts, and then +page.ts for route config options, taking the first instance it finds. It does not merge them. In the case of entries, I suppose it's not impossible for us to just grab all of the instances along that chain, run them, and merge the result, but that seems like supporting an antipattern to me -- there's no conceivable reason I can think of that you'd need to export multiple instances of entries from the same route (cc @Rich-Harris, what do you think?).

I think the action items here are:

  • Figure out why .svelte-kit isn't prerendering 1 in the first part of this issue
  • Throw an informative error when we detect multiple instances of entries in the same route. Or merge them, though I'm not in favor of this -- not for its complexity, but just because it seems better to lean on convention here.

Created a reproduction repository. Please check the commits that start with the message demo:.

System:
  OS: macOS 13.3.1
  CPU: (8) arm64 Apple M1
  Memory: 84.09 MB / 8.00 GB
  Shell: 5.9 - /bin/zsh

Binaries:
  Node: 18.16.0 - ~/.nvm/versions/node/v18.16.0/bin/node
  npm: 9.5.1 - ~/.nvm/versions/node/v18.16.0/bin/npm

Browsers:
  Chrome: 113.0.5672.92
  Firefox: 112.0.1
  Safari: 16.4

npmPackages:
  @sveltejs/adapter-auto: ^2.0.0 => 2.0.1 
  @sveltejs/kit: ^1.5.0 => 1.16.3 
  svelte: ^3.54.0 => 3.59.1 
  vite: ^4.3.0 => 4.3.6 

hyunbinseo avatar May 16 '23 05:05 hyunbinseo

For additional context,

I have stumbled upon this issue in the following real-world example. Reproduction

/src/routes/[artist]/[release]

# Both of these pages (artist and artist's release)
/younha
/younha/end-theory

# Request the following JSON endpoint using content negotiation.
# https://kit.svelte.dev/docs/routing#server-content-negotiation
/younha

Note that the pre-rendering is disabled in src/routes/[artist]/[release]/+page.ts.

// Error: Cannot save /younha/end-theory-final-edition as /younha is already a file.
export const prerender = false;

I believe fixing the first part of the issue will fix this reproduction as well.

Path Dev Preview
/younha Page Rendered JSON
/younha/ Redirected to above 404 Not Found
/younha/end-theory-final-edition Page Rendered Page Rendered

The above behavior is expected, since the following files are generated.

tree .svelte-kit/output/prerendered

.svelte-kit/output/prerendered
└── pages
    └── younha # pre-rendered +server.ts

2 directories, 1 file

Worked around this issue by renaming the server endpoint as guided.

- src/routes/[artist]/+server.ts
+ src/routes/[artist].json/+server.ts

hyunbinseo avatar May 16 '23 07:05 hyunbinseo

Okay, just categorizing a few things here for notes:

  • demo: only /pages/2 is pre-rendered
    • The fact that only /pages/2 is prerendered is expected; only one entries per route is evaluated. Priority goes from the server to the client (+server => +page.server => +page)
  • demo: /output/prerendered not generated
    • Appears to be a bug -- /pages/2.html should be prerendered
  • demo: only /pages/1 is pre-rendered
    • Correct behavior

Investigating cause :loading:

Figured it out! Only took two hours :upside-down-cowboy:

I made an error earlier (I think? we'll see what Rich says) -- the precedence of entries was incorrectly +server => +page => +page.server. It should be +page => +page.server => +server. This'll be fixed in the same PR.

I noticed a potential separate issue here, though, which I'll file a separate issue about.

Update: New issue has been created. https://github.com/sveltejs/kit/issues/9995


It would be nice if these information are documented.

  • Only one entries is evaluated per route.
  • The priorities are (+page > +page.server > +server).

Providing relevant type or build errors would be nice if multiple entries are exported.

hyunbinseo avatar May 20 '23 05:05 hyunbinseo

Yep, I'm making a followup issue for throwing an error if multiple entries are specified

Per https://github.com/sveltejs/kit/pull/9994#issuecomment-1559428738, I think we should just error if +page.svelte (and/or +page.js/+page.server.js) and +server.js both exist for a prerenderable route. I can't think of a valid reason to have both. Am I missing something?

Rich-Harris avatar May 23 '23 13:05 Rich-Harris

I agree that multiple entries should not be exported from a single route.

Not because there are no use cases for it - reference https://github.com/sveltejs/kit/issues/9929#issuecomment-1549117741

├── +page.svelte
├── +page.ts # Fetches the following JSON endpoint using content negotiation
└── +server.ts # Shared, common JSON endpoint that can be used by other routes

but because content negotiation cannot be done using simple static file serving.

For example in Cloudflare Pages, it will require Workers invocation which is not free.

I just hope that the error is shown before build to avoid last minute fixes.

hyunbinseo avatar May 23 '23 15:05 hyunbinseo

Closed via #11256, having conflicting route definitions / both a +page.js and +server.js when prerendering is an error in SvelteKit 2.0

dummdidumm avatar Dec 13 '23 09:12 dummdidumm