test for bug reproduction

[alexanderniebuhr]

@matthewp this adds a test to reproduce the issue:

1. pnpm i && pnpm run build
2. node --test "packages/astro/test/ssr-prerender-chunk.test.js"
3. Open packages/astro/test/fixtures/ssr-prerender-chunks/dist/_worker.js/renderers.mjs
4. Follow the imports from that file import { r as reactExports, R as React } from './chunks/prerender_dvgq6Avf.mjs';
5. Open packages/astro/test/fixtures/ssr-prerender-chunks/dist/_worker.js/chunks/prerender_dvgq6Avf.mjs
6. See that the chunking tries to import from a file with noop content

It is not possible to test this with the exisiting testAdapter, since that relies on fs and also doesn't use a real serverEntrypoint but a Vite virtual import. I expect this is an issue with manualChunks settings of Astro core, but it might could be also an issue with some settings for the serverless-test-adapter. Still I'm very lost and would love to get help & or a fix!

(Maybe @bluwy has also an idea 🤔)

As my debugging goes the following issues are downstream issues caused by this:

• Cloudflare v10 + Solid breaks some components and scripts adapters#224
• Cloudflare v10 breaks cloudflare build / wrangler preview with React adapters#211
• QRCode Issues adapters#233
• Cloudflare v10 navigating to a non-existent page (404) with React causes react minified errors with Wrangler adapters#214

[changeset-bot]

⚠️ No Changeset found

Latest commit: f95ace9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[bluwy]

I looked into this in the past couple days, and I think it's hard to fix this. I've tried:

1. Use this.emitFile() with preserveSignature: 'strict' so that the prerender chunk doesn't contain and export unrelated modules, but it wasn't possible to ensure all prerendered pages to go into this chunk. manualChunks could've kept some chunks out of it.
2. Instead of making the entire prerender chunk exports a noop, try to analyze the Astro component exports and noop them only. However, it's not possible to easily analyze this with Rollup or es-module-lexer without making assumptions of the chunk code and manually treeshake it.

At the end, both approaches aren't perfect either, and I'm not sure if it's worth making them work. I think the more proper solution for now is to:

1. Remove the prerender chunk and noop flow. (Accept that this brings a larger bundle size)
2. Each page should correspond to a Rollup input and contain its page code within it. The former is true today, but the latter is not because each input correspond to (yet another?) dynamic entry chunk:

   astro/packages/astro/src/core/build/plugins/plugin-pages.ts

   Lines 47 to 56 in f95ace9

   	const resolvedPage = await this.resolve(pageData.moduleSpecifier);
   	if (resolvedPage) {
   	imports.push(`const page = () => import(${JSON.stringify(pageData.moduleSpecifier)});`);
   	exports.push(`export { page }`);
   	imports.push(`import { renderers } from "${RENDERERS_MODULE_ID}";`);
   	exports.push(`export { renderers };`);
   	return `${imports.join('\n')}${exports.join('\n')}`;
   	}

3. That way we can safely delete the input chunks of the prerendered pages only. These input chunks should never be used again.

In the future, an even proper solution would be to run two rollup builds: SSG/prerendered pages and SSR/dynamic pages. It would be tricky to do that now because:

1. export const prerender can be tied to env vars, we don't know ahead of time if the page should be prerendered.
2. Two Rollup builds may need different configuration by users and adapters (Vite 6 environment API may help).
3. Two Rollup builds is likely to be slow.

[alexanderniebuhr]

Thanks @bluwy for looking into this. I do agree with your outcome since I discovered something similar.

I like both ideas, not using the prerender chunk noop flow and having two builds. Do you think it would be possible to add those workarounds now (4.11 maybe) and then come back to this with vite 6 and environment api?

I also tried adding a flag, but not super happy, but it kinda allows for two builds: #11155

What do you think about that?

[alexanderniebuhr]

Also very interested how @matthewp thinks about this, and which approach he would like to go, I just really think we should find a solution now and then look at a long term fix later :)

[matthewp]

I'd be fine with getting rid of the noop for now and thinking about another build later. There's a chance we'll need another build when we do server islands (although i hope not) so i could see changing how it works then.

[bluwy]

I think we can definitely do what's described at I think the more proper solution for now is to: ... for sure. I don't think they will take as much work but still need a little grease. I can try to attempt step 1-3 next week, but if they take more work, doing step 1 only should be fine.

The two builds idea is something we would/could do far in the future though, so not something we should do now but can think about.

[alexanderniebuhr]

Sounds great for me. If you're trying to do Step 1-3 next week, I'll be around and be able to test a preview release with the Cloudflare adapter to see if works :) ❤️