Omit env from server renders if $env/dynamic/public isn't used

[Rich-Harris]

Describe the problem

Every public environment variable is made available to client-side code via $env/dynamic/public in the init block:

<script type="module" data-sveltekit-hydrate="gb5305">
  import { start } from "../_app/immutable/start-02c2bf02.js";

  start({
    assets: "",
    env: {
      PUBLIC_FOO: 'a potentially very long string',
      PUBLIC_BAR: 'another potentially very long string',
      PUBLIC_BAZ: 'yet another potentially very long string. i could do this all day'
    },
    // ...
  });
</script>

But many apps don't actually use $env/dynamic/public, they use $env/static/public instead (which in general is preferred).

Describe the proposed solution

Detect cases where $env/dynamic/public is not imported by client-side code, and omit public env vars from the init block.

Alternatives considered

No response

Importance

nice to have

Additional Information

No response

[ghostdevv]

What would be the best way to check whether $env/dynamic/public is used?

[Rich-Harris]

In dev we'd just assume it was always used, because we can't really know. At build time we can inspect the module graph to know which modules are imported, though drawing the rest of the owl is finicky and I always struggle with it a bit.

We're doing something similar already to ensure that you don't accidentally import $env/(static|dynamic)/private here:

kit/packages/kit/src/exports/vite/index.js

Lines 370 to 397 in 98226de

	/**
	* Ensures that client-side code can't accidentally import server-side code,
	* whether in `*.server.js` files, `$lib/server`, or `$env/[static|dynamic]/private`
	* @type {import('vite').Plugin}
	*/
	const plugin_guard = {
	name: 'vite-plugin-sveltekit-guard',
	writeBundle: {
	sequential: true,
	async handler(_options) {
	if (vite_config.build.ssr) return;
	const guard = module_guard(this, {
	cwd: vite.normalizePath(process.cwd()),
	lib: vite.normalizePath(kit.files.lib)
	});
	manifest_data.nodes.forEach((_node, i) => {
	const id = vite.normalizePath(
	path.resolve(kit.outDir, `generated/client-optimized/nodes/${i}.js`)
	);
	guard.check(id);
	});
	}
	}
	};

We can probably reuse the module_guard logic somehow to avoid traversing the graph more than we need to — maybe it stores a list of all imported modules or something that we can inspect later. The guard logic happens during the client build that runs here here:

kit/packages/kit/src/exports/vite/index.js

Line 630 in 98226de

await vite.build({

Because the plugin gets reinstantiated for the client build, I think we'd need to store that list of modules in a variable the same way we store secondary_build_started. Then we need to include information about whether the list includes $env/dynamic/public in the SSRManifest that gets generated here...

kit/packages/kit/src/exports/vite/index.js

Lines 661 to 669 in 98226de

	// regenerate manifest now that we have client entry...
	fs.writeFileSync(
	manifest_path,
	`export const manifest = ${generate_manifest({
	build_data,
	relative_path: '.',
	routes: manifest_data.routes
	})};\n`
	);

so that render.js can omit the script block when it generates HTML.

All of which sounds terribly convoluted — we're building a server so that we can analyse it in order to create an optimised client build, which we then analyse so that we can optimise the server — but I think it'll work.

[Rich-Harris]

I'm looking into #8997, and we might actually not want to do this. Am investigating option 2, wherein requests for $env/dynamic/public are remapped to requests for /_app/env.js which is dynamically generated. In that case setting env upon render would be unnecessary.

I'm not sure if this the best approach, since it won't work in cases where paths.assets is set, but in any case I think we should hold off on the above for the moment.

[Rich-Harris]

Reverse ferret: turns out we can't do the /_app/env.js thing, because that would be a breaking change. So analysing the bundle is back on the table

[jonshipmansmc]

Is this issue solved in SvelteKit2 @Rich-Harris