feat: remove singleFetch flag by jacob-ebey · Pull Request #11522 · remix-run/react-router

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

  });

-  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

github-actions · 2025-01-14T16:11:08Z

🤖 Hello there,

We just published version 6.28.2-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

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

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

brophdawg11 mentioned this pull request Apr 29, 2025

Fix window error in PrefetchPageLinks during SSR in Single Fetch remix-run/remix#10601

Merged

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

Uh oh!

Conversation

jacob-ebey commented May 2, 2024

Uh oh!

changeset-bot bot commented May 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot bot commented May 2, 2024 •

edited

Loading