Skip to content

Commit

Permalink
fix(dev): pass script props to deferred inline script tags (#6389)
Browse files Browse the repository at this point in the history
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
  • Loading branch information
jack-r-warren and kentcdodds committed Jun 14, 2023
1 parent eb06147 commit 4efe65f
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 2 deletions.
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

0 comments on commit 4efe65f

Please sign in to comment.