Make <Scripts> render nothing on the client

[developit]

This makes <Scripts> do nothing on the client. In Preact, the mismatched <script> and <link> tags in the SSR'd HTML will be skipped over during hydration, and I'm fairly certain this is also the case in React.

The existing implementation could be further simplified now that useMemo() is no longer needed to prevent reinsertion of scripts after hydration. Also, if there's a compile-time equivalent of the typeof document !== 'undefined' check (process.browser is a common constant in build systems), that would make it possible to avoid shipping <Scripts> to the client at all.

[remix-cla-bot]

Hi @developit,

Welcome, and thank you for contributing to Remix!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected].

Thanks!

- The Remix team

[remix-cla-bot]

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

[sergiodxa]

One way to don't ship Script to browser would be to create a folder with the server and browser implementations and a package.json with the main and browser keys pointing to each one.

[ryanflorence]

I think this would be performance regression because <Scripts> renders <link rel="modulepreload"> for all of the next matches when a transition starts.

remix/packages/remix-react/components.tsx

Lines 755 to 776 in 2facf69

	let routePreloads = matches
	.concat(nextMatches)
	.map(match => {
	let route = manifest.routes[match.route.id];
	return (route.imports || []).concat([route.module]);
	})
	.flat(1);
	let preloads = manifest.entry.imports.concat(routePreloads);
	return (
	<>
	{dedupe(preloads).map(path => (
	<link
	key={path}
	rel="modulepreload"
	href={path}
	crossOrigin={props.crossOrigin}
	/>
	))}
	{initialScripts}
	</>

Without that, when we dynamically import() the next route module we'll have an import waterfall of all of its dependencies, but the way the code is today the entire module graph for the next page starts preloading in parallel even before we import() the next routes.

So it's not just a server thing, it matters for client transitions too.

We already memoize the initial <scripts> that are only needed on the server and they never change after the initial render:

remix/packages/remix-react/components.tsx

Lines 702 to 741 in 2facf69

	let initialScripts = React.useMemo(() => {
	// if (typeof document !== 'undefined') return null;
	let contextScript = serverHandoffString
	? `window.__remixContext = ${serverHandoffString};`
	: "";
	let routeModulesScript = `${matches
	.map(
	(match, index) =>
	`import * as route${index} from ${JSON.stringify(
	manifest.routes[match.route.id].module
	)};`
	)
	.join("\n")}
	window.__remixRouteModules = {${matches
	.map((match, index) => `${JSON.stringify(match.route.id)}:route${index}`)
	.join(",")}};`;
	return (
	<>
	<script
	{...props}
	suppressHydrationWarning
	dangerouslySetInnerHTML={createHtml(contextScript)}
	/>
	<script {...props} src={manifest.url} />
	<script
	{...props}
	dangerouslySetInnerHTML={createHtml(routeModulesScript)}
	type="module"
	/>
	<script {...props} src={manifest.entry.module} type="module" />
	</>
	);
	// disabled deps array because we are purposefully only rendering this once
	// for hydration, after that we want to just continue rendering the initial
	// scripts as they were when the page first loaded
	// eslint-disable-next-line
	}, []);

So it's not clear to me what problem we have and what this PR is attempting to solve? Please re-open (but include the module preloads ofc) if I'm missing something :)