feat: remove singleFetch flag #11522

jacob-ebey · 2024-05-02T18:08:44Z

No description provided.

changeset-bot · 2024-05-02T18:08:47Z

🦋 Changeset detected

Latest commit: 6c1e2d8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages

Name	Type
@react-router/server-runtime	Major
react-router-dom	Major
@react-router/express	Major
react-router	Major
@react-router/serve	Major
@react-router/node	Major
@react-router/dev	Major

Not sure what this means? Click here to learn what changesets are.

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

jacob-ebey · 2024-05-03T20:26:00Z

integration/resource-routes-test.ts

@@ -208,7 +208,9 @@ test.describe("loader in an app", async () => {
    expect(await res.text()).toEqual("Partial");
  });

-  test("should handle objects returned from resource routes", async ({
+  // TODO: Matt and I chatted about this recently, revisit that chat and


@brophdawg11 I'm disabling this and we can update / re-enable it after your PR goes in.

yeah this should work after remix-run/remix#9349 is brought over - wanted to do that after this was merged to avoid conflicts

jacob-ebey · 2024-05-03T20:28:49Z

packages/react-router/__tests__/dom/ssr/components-test.tsx

-      !(staticHandlerContext instanceof Response),
-      "Expected a context"
-    );
+// TODO: figure out why this test exists


I'm not sure why these exist but the underlying issue causing them to fail should probably be fixed.

These tests were from an issue a while back where IIRC the lack of a default export was being converted to export default {} in the compiler and that was causing us to think it had a component export and eventually call createElement({}) - so we strengthened our check for a component to make sure it was an actual react component. I don't specifically recall if they were esbuild only or also a vite thing though.

I got the tests passing and pushed it up

remix-run/remix#7593 (comment)
https://github.com/remix-run/remix/blob/main/packages/remix-react/routes.tsx#L614
remix-run/remix#7745

brophdawg11 · 2024-05-08T12:13:41Z

packages/react-router/lib/dom/ssr/components.tsx

-    return `__remixContext.r(${JSON.stringify(routeId)}, ${JSON.stringify(
-      key
-    )}, ${escapeHtml(serializedData)})`;
-  };


brophdawg11 · 2024-05-08T12:16:05Z

packages/react-router/lib/dom/ssr/single-fetch.tsx

+      ? new URL(
+          reqUrl,
+          typeof window === "undefined"
+            ? // TODO: Trace usage of singleFetchUrl and make sure we don't use it anywhere
+              // this will make it to a network call.
+              "server://singlefetch/"
+            : window.location.origin
+        )


Do you recall what needed this? I removed it and things seem to be passing...

Since it's part of the DOM code it should always generally be running in a jsdom environment I think?

I removed this in d16ec89 - feel free to revert though if we need to keep it

found the one integration test that was failing - PrefetchPageLinks calls it during SSR so I added it back with a comment. The other 2 uses of it do result in a network call but both are in the browser so window will be available.

brophdawg11 · 2024-05-08T12:17:34Z

packages/remix-dev/config/defaults/entry.server.spa.tsx

@@ -1,20 +1,73 @@
-import type { EntryContext } from "@react-router/node";
+import { PassThrough } from "node:stream";


I think we can just remove this file entirely and the condition that uses it in the remix dev and remix reveal code?

https://github.com/remix-run/react-router/blob/v7/packages/remix-dev/config.ts#L493
https://github.com/remix-run/react-router/blob/v7/packages/remix-dev/cli/commands.ts#L148

Ah I see what you did - you got rid of the prohibitOutOfOrderStreaming check and just made the spa version always use handleBotRequest. IMO, it would be fine to just delete the SPA version and let npx remix reveal expose the default node one with the prohibitOutOfOrderStreaming check

Took a stab at that in dd6a86c - feel free to revert if we want to keep it 👍

hm - so that broke a handful of e2e tests so I reverted for now :)

brophdawg11 · 2024-05-08T12:27:12Z

packages/remix-server-runtime/server.ts

-    }
-  } else {
-    statusCode = context.statusCode;
-    headers = getDocumentHeaders(build, context);


I also removed the now unused headers.ts file and the set-cookie-parser dependency in 531b286

This reverts commit dd6a86c.

This reverts commit d16ec89.

chore: add changeset

remix-cla-bot bot added the CLA Signed label May 2, 2024

jacob-ebey added 2 commits May 3, 2024 09:22

feat: remove singleFetch flag

3a23492

remove another reference

0907f61

jacob-ebey force-pushed the remove_single_fetch_flag branch from 6fc780d to 0907f61 Compare May 3, 2024 16:28

chore: update unit tests

78d626c

jacob-ebey commented May 3, 2024

View reviewed changes

jacob-ebey and others added 3 commits May 7, 2024 11:54

fix: spa mode tests

3120dea

update resource tests

2fd839d

fix commented out test

873c767

brophdawg11 reviewed May 8, 2024

View reviewed changes

brophdawg11 added 3 commits May 8, 2024 08:24

Remove server://singlefetch origin stub

d16ec89

Remove unused headers.ts file and set-cookie-parser dependencies

531b286

Remove entry.server.spa.tsx

dd6a86c

brophdawg11 reviewed May 8, 2024

View reviewed changes

brophdawg11 and others added 7 commits May 8, 2024 08:48

Remove/adjust a few remaining _data references in tests

8840269

Turn off integration test debug logs

349211b

Remove unused _data request code paths

24826aa

Revert "Remove entry.server.spa.tsx"

1113982

This reverts commit dd6a86c.

Revert "Remove server://singlefetch origin stub"

30dd74d

This reverts commit d16ec89.

Update comment for clarity

e084eda

format

f9c1286

brophdawg11 approved these changes May 9, 2024

View reviewed changes

jacob-ebey marked this pull request as ready for review May 9, 2024 18:47

chore: ignore playgrounds from changesets

6c1e2d8

chore: add changeset

jacob-ebey merged commit aab4ea0 into v7 May 9, 2024
8 checks passed

jacob-ebey deleted the remove_single_fetch_flag branch May 9, 2024 18:52

github-actions bot mentioned this pull request Sep 19, 2024

chore: Update version for release (pre) #12016

Merged

github-actions bot mentioned this pull request Nov 22, 2024

chore: Update version for release #12344

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: remove singleFetch flag #11522

feat: remove singleFetch flag #11522

jacob-ebey commented May 2, 2024

changeset-bot bot commented May 2, 2024 •

edited

Loading

jacob-ebey May 3, 2024

brophdawg11 May 8, 2024

jacob-ebey May 3, 2024

brophdawg11 May 8, 2024

brophdawg11 May 8, 2024

brophdawg11 May 8, 2024

brophdawg11 May 8, 2024

brophdawg11 May 8, 2024

brophdawg11 May 8, 2024

brophdawg11 May 8, 2024

brophdawg11 May 8, 2024

brophdawg11 May 8, 2024

brophdawg11 May 8, 2024

		@@ -1,20 +1,73 @@
		import type { EntryContext } from "@react-router/node";
		import { PassThrough } from "node:stream";

feat: remove singleFetch flag #11522

feat: remove singleFetch flag #11522

Conversation

jacob-ebey commented May 2, 2024

changeset-bot bot commented May 2, 2024 • edited Loading

🦋 Changeset detected

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

changeset-bot bot commented May 2, 2024 •

edited

Loading