Skip to content
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

fix(dev): pass script props to deferred inline script tags #6389

Merged
merged 9 commits into from
Jun 14, 2023
5 changes: 5 additions & 0 deletions .changeset/two-cobras-float.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/react": patch
---

properly pass <Scripts /> props to inline script tags for deferred data
1 change: 1 addition & 0 deletions contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@
- jacargentina
- jacob-ebey
- JacobParis
- jack-r-warren
- jakeginnivan
- jakewtaylor
- jamiebuilds
Expand Down
102 changes: 102 additions & 0 deletions packages/remix-react/__tests__/deferred-scripts-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import * as React from "react";
import { createMemoryRouter } from "react-router-dom";
import { defer } from "react-router-dom";
import { StaticRouterProvider } from "react-router-dom/server";
import { render } from "@testing-library/react";
import type { EntryContext } from "@remix-run/server-runtime";

import { Scripts, RemixContext } from "../components";

import "@testing-library/jest-dom/extend-expect";

/**
* Don't try to add additional tests for <Scripts /> to this file!
*
* <Scripts /> gates most of its functionality on the **unexported**
* variable isHydrated, defined inside ../components, and it sets that
* variable the first time it is rendered. This means that subsequent
* calls to <Scripts /> will skip writing actual script tags.
*
* Using jest.resetModules() or .isolateModules() won't help here as
* we'd need to re-import or re-require [1]. Re-importing would need new
* packages to work [2] and re-requiring has problems with React [3].
*
* [1]: https://github.com/jestjs/jest/issues/3236
* [2]: https://github.com/jestjs/jest/issues/3236#issuecomment-698271251
* [3]: https://github.com/jestjs/jest/issues/11471
*/

describe("<Scripts /> with activeDeferreds", () => {
it("should pass custom props", () => {
let context: EntryContext = {
routeModules: { root: { default: () => null } },
manifest: {
routes: {
root: {
hasLoader: false,
hasAction: false,
hasCatchBoundary: false,
hasErrorBoundary: false,
id: "root",
module: "root.js",
},
},
entry: { imports: [], module: "" },
url: "",
version: "",
},
// @ts-expect-error
// Enumerating all flags just to set them to false is overly brittle;
// we allow an empty object as a shortcut.
future: {},
// @ts-expect-error
// Similarly, we have no interest in the rest of the static handler
// context. We're not trying to write a test for React Router, we
// just want to trick <Scripts /> into thinking there's a need for
// deferred scripts. We'll look for "key with a promise" to check that
// this isn't being ignored.
staticHandlerContext: {
activeDeferreds: {
"/": defer({
"key with a promise": new Promise((resolve) => resolve("value")),
}),
},
},
};

let router = createMemoryRouter([
{
id: "root",
path: "/",
element: <Scripts nonce="some nonce" />,
},
]);

let { container } = render(
<RemixContext.Provider value={context}>
<StaticRouterProvider
router={router}
context={context.staticHandlerContext}
// We're testing <Scripts />, but <StaticRouterProvider /> can insert
// its own script tags too, so we pass it the same nonce prop so our
// check for "do all script tags have the nonce prop" still works.
nonce="some nonce"
/>
</RemixContext.Provider>
);

let scriptElements = Array.from(container.getElementsByTagName("script"));

// Confirm that activeDeferreds is being handled
expect(
scriptElements.filter((elem) =>
elem.innerHTML.includes("key with a promise")
).length
).toBeGreaterThan(0);

// Test that all script tags have the additional nonce prop
scriptElements.forEach((elem) =>
expect(elem).toHaveAttribute("nonce", "some nonce")
);
});
});
13 changes: 11 additions & 2 deletions packages/remix-react/components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,7 @@ export function Scripts(props: ScriptProps) {
deferredData={deferredData}
routeId={routeId}
dataKey={key}
scriptProps={props}
/>
);

Expand Down Expand Up @@ -1048,7 +1049,7 @@ import(${JSON.stringify(manifest.entry.module)});`;

if (!isStatic && typeof __remixContext === "object" && __remixContext.a) {
for (let i = 0; i < __remixContext.a; i++) {
deferredScripts.push(<DeferredHydrationScript key={i} />);
deferredScripts.push(<DeferredHydrationScript key={i} scriptProps={props} />);
}
}

Expand Down Expand Up @@ -1102,10 +1103,12 @@ function DeferredHydrationScript({
dataKey,
deferredData,
routeId,
scriptProps,
}: {
dataKey?: string;
deferredData?: DeferredData;
routeId?: string;
scriptProps?: ScriptProps;
}) {
if (typeof document === "undefined" && deferredData && dataKey && routeId) {
invariant(
Expand All @@ -1125,6 +1128,7 @@ function DeferredHydrationScript({
dataKey &&
routeId ? null : (
<script
{...scriptProps}
async
suppressHydrationWarning
dangerouslySetInnerHTML={{ __html: " " }}
Expand All @@ -1136,10 +1140,11 @@ function DeferredHydrationScript({
<Await
resolve={deferredData.data[dataKey]}
errorElement={
<ErrorDeferredHydrationScript dataKey={dataKey} routeId={routeId} />
<ErrorDeferredHydrationScript dataKey={dataKey} routeId={routeId} scriptProps={scriptProps} />
}
children={(data) => (
<script
{...scriptProps}
async
suppressHydrationWarning
dangerouslySetInnerHTML={{
Expand All @@ -1154,6 +1159,7 @@ function DeferredHydrationScript({
/>
) : (
<script
{...scriptProps}
async
suppressHydrationWarning
dangerouslySetInnerHTML={{ __html: " " }}
Expand All @@ -1166,9 +1172,11 @@ function DeferredHydrationScript({
function ErrorDeferredHydrationScript({
dataKey,
routeId,
scriptProps,
}: {
dataKey: string;
routeId: string;
scriptProps?: ScriptProps;
}) {
let error = useAsyncError() as Error;
let toSerialize: { message: string; stack?: string } =
Expand All @@ -1184,6 +1192,7 @@ function ErrorDeferredHydrationScript({

return (
<script
{...scriptProps}
suppressHydrationWarning
dangerouslySetInnerHTML={{
__html: `__remixContext.r(${JSON.stringify(routeId)}, ${JSON.stringify(
Expand Down