Skip to content

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

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
hyunbinseo opened this issue May 15, 2023 · 10 comments
Closed

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

hyunbinseo opened this issue May 15, 2023 · 10 comments
Assignees
Milestone

Comments

@hyunbinseo
Copy link
Contributor

hyunbinseo commented May 15, 2023

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

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
@elliott-with-the-longest-name-on-github
Copy link
Contributor

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.

@hyunbinseo
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@elliott-with-the-longest-name-on-github
Copy link
Contributor

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:

@elliott-with-the-longest-name-on-github
Copy link
Contributor

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.

@hyunbinseo
Copy link
Contributor Author

hyunbinseo commented May 20, 2023

Update: New issue has been created. #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.

@elliott-with-the-longest-name-on-github
Copy link
Contributor

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

@Rich-Harris
Copy link
Member

Per #9994 (comment), 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?

@hyunbinseo
Copy link
Contributor Author

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

Not because there are no use cases for it - reference #9929 (comment)

├── +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.

@dummdidumm dummdidumm added this to the 2.0 milestone Dec 13, 2023
@dummdidumm
Copy link
Member

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

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 a pull request may close this issue.

4 participants