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

Map and Set entries reversed on page hydration #13093

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

bjohn465
Copy link

@bjohn465 bjohn465 commented Feb 22, 2025

When the loader data contains a Map or Set object, the data maintains the correct order in the server-rendered response. However, once the page hydrates, the order of the items in the Map or Set are reversed, leading to a hydration error from React. This also breaks the specifications for Map and Set, since they specify that the iteration order of the entries should match the insertion order.

This is a failing bug report test, demonstrating this behavior.

When the loader data contains a `Map` or `Set` object,
the data maintains the correct order
in the server-rendered response.

However, once the page hydrates,
the order of the items in the `Map` or `Set` are reversed,
leading to a hydration error from React.
This is me signing the Contributor License Agreement (CLA).
Copy link

changeset-bot bot commented Feb 22, 2025

⚠️ No Changeset found

Latest commit: 06e8912

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

So we are less likely
to accidentally match text content
that appears elsewhere in the page markup.
@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Feb 23, 2025

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

@bjohn465
Copy link
Author

bjohn465 commented Feb 23, 2025

I tried looking into this and here is what I found:

The body stream passed to the decodeViaTurboStream function in packages/react-router/lib/dom/ssr/single-fetch.tsx has what appears to be a turbo-stream-encoded Map in the correct order:

[{"_1":2,"_9":-5,"_10":-5},"loaderData",{"_3":4,"_8":-5},"routes/map",["M",5,5,6,6,7,7],1,2,3,"root","actionData","errors"]

(The ["M",5,5,6,6,7,7] would be the encoded Map, and it references items at indexes 5, 6, and 7, which are the values 1, 2, and 3. It references each value twice—once for the key and once for the value.)

After the call to turbo-stream's decode method resolves, however, the Map entries are reversed in order.

So this could be an issue with the version of turbo-stream being used. I did not look into that any further, though.

@bjohn465
Copy link
Author

bjohn465 commented Feb 23, 2025

It looks like this is caused by the version of turbo-stream currently in use. I tried encoding and decoding a Map using turbo-stream v2.4.1 and the order of the entries was reversed. Using turbo-stream v3.1.0, encoding and decoding a Map results in the correct order of entries.

I see that there's a pull request to upgrade turbo-stream to v3.10. I'll keep this pull request open for the tests at least.

Since the turbo-stream upgrade is going to be a major change (the turbo-stream format is different in v3 versus v2), some people may not be able to upgrade right away. I don't know if the maintainer(s) of turbo-stream would be open to releasing another v2 version that would fix this issue. I opened a turbo-stream issue asking about that possibility. If not, maybe maintaining a v2 fork would be helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant