kit icon indicating copy to clipboard operation
kit copied to clipboard

feat: append prerendered redirects to `_redirects` file

Open Zhincore opened this issue 1 year ago • 1 comments

Makes Cloudflare Pages adapter generate a static _redirects file in build output. This way the redirects happen on server and don't have to be a client-side HTML file as it is now.

It simply collects redirects from builder.prerendered.redirects, so I am not sure how reliable that is?

My example purpose is that I have following +server.ts file:

import { redirect } from '@sveltejs/kit';
import path from "$data/file.json?url"
import type { RequestHandler } from './$types';

export const prerender = true;

export const GET: RequestHandler = async () => {
    redirect(302, path);
};

Normally the adapter generates a __data.json and an .html file that redirects the user there (which still happens, I didn't change that), but I need that redirect to happen on server, so that the file.json file is served.


Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • [ ] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • [x] This message body should clearly illustrate what problems it solves.
  • [ ] Ideally, include a test that fails without this PR but passes with it.

Tests

  • [x] Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

Zhincore avatar May 08 '24 15:05 Zhincore

🦋 Changeset detected

Latest commit: be8b4bb21b3788002b6841c58dea4ce5eebbf828

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

This PR includes changesets to release 1 package
Name Type
@sveltejs/adapter-cloudflare Minor

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

changeset-bot[bot] avatar May 08 '24 15:05 changeset-bot[bot]

If the target of redirect is a small file, it gets included in the redirects as data:base64 URL (not valid in Cloudflare's eyes), is there a way to prevent it, apart form changing Vite's inline limit?

Due to the latest change, client gets served the HTML/JS redirect instead of being redirect by the server.

Zhincore avatar Jul 01 '24 14:07 Zhincore

If the target of redirect is a small file, it gets included in the redirects as data:base64 URL (not valid in Cloudflare's eyes), is there a way to prevent it, apart form changing Vite's inline limit?

Not that I know of. Your best bet is to add a predicate to the build.assetsInlineLimit option so it returns false when it matches the target filepath https://vitejs.dev/config/build-options.html#build-assetsinlinelimit

Due to the latest change, client gets served the HTML/JS redirect instead of being redirect by the server.

Which change are you referring to? Can you elaborate?

teemingc avatar Jul 01 '24 16:07 teemingc

The predicate might be a good thing to be mentioned in the docs if this PR gets merged.

Which change are you referring to? Can you elaborate?

I'm referring to my own commit which excludes the redirected path from being processed by the worker

Zhincore avatar Jul 01 '24 16:07 Zhincore

Which change are you referring to? Can you elaborate?

I'm referring to my own commit which excludes the redirected path from being processed by the worker

I'm not sure if I understand. Isn't the _redirects rule serving the redirect instead of the HTML/JS file?

teemingc avatar Jul 01 '24 16:07 teemingc

I'm not sure if I understand. Isn't the _redirects rule serving the redirect instead of the HTML/JS file?

Yes, exactly, but when the files gets inlined, it gets inlined inside the _redirects as well (it's a data: URL instead of a path to file). Cloudflare ignores that rule as invalid and serves the static HTML instead.

Zhincore avatar Jul 01 '24 16:07 Zhincore

I'm not sure if I understand. Isn't the _redirects rule serving the redirect instead of the HTML/JS file?

Yes, exactly, but when the files gets inlined, it gets inlined inside the _redirects as well (it's a data: URL instead of a path to file). Cloudflare ignores that rule as invalid and serves the static HTML instead.

Weird, I wasn't able to reproduce that. When I followed the +server.js example from the PR description, there simply wasn't any HTML file reproduced and the _redirects file was empty. Maybe we can throw an error with instructions on how to fix this if one of the redirect locations is a base64 string (I'd open a new issue and PR for this if the inlined base64 string causes the prerendered redirect to disappear because it sounds different from the scope of this PR).

Side note: we should probably add a check to see if there are any prerendered redirects before writing to _redirects:

+ if (builder.prerendered.redirects.size > 0) {
	writeFileSync(`${dest}/_redirects`, generate_redirects(builder), { flag: 'a' });
+ }

teemingc avatar Jul 01 '24 16:07 teemingc

Thank you!

teemingc avatar Jul 23 '24 19:07 teemingc