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

Compare name when hydrating hidden fields to filter out extra form action fields #26846

Merged
merged 1 commit into from
May 26, 2023

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented May 24, 2023

This solves an issue where if you inject a hidden field in the beginning of the form, we might mistakenly hydrate the injected one that was part of an action.

I'm not too happy about how specific this becomes. It's similar to Float but in general we don't do this deep comparison.

See vercel/next.js#50087

@sebmarkbage sebmarkbage requested a review from gnoff May 24, 2023 21:22
@facebook-github-bot facebook-github-bot added the React Core Team Opened by a member of the React Core Team label May 24, 2023
Copy link
Collaborator

@gnoff gnoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes getting quite specific. Since these fields are implementation details and not really in the purview of app authors maybe this is a case where we do some data attribute to mark skippable. won't really make the code smaller and will make payloads slightly bigger but makes the comparison feel less arbitrarily specific

@sebmarkbage sebmarkbage force-pushed the hydratehidden branch 3 times, most recently from 7a054c9 to 9622750 Compare May 25, 2023 21:19
@react-sizebot
Copy link

Comparing: 4daccad...f216bac

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 164.23 kB 164.23 kB = 51.77 kB 51.77 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js +0.04% 171.59 kB 171.67 kB +0.05% 53.98 kB 54.01 kB
facebook-www/ReactDOM-prod.classic.js = 570.22 kB 570.22 kB = 100.63 kB 100.63 kB
facebook-www/ReactDOM-prod.modern.js = 553.95 kB 553.95 kB = 97.81 kB 97.81 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against f216bac

@sebmarkbage sebmarkbage merged commit a1f9758 into facebook:main May 26, 2023
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…tion fields (facebook#26846)

This solves an issue where if you inject a hidden field in the beginning
of the form, we might mistakenly hydrate the injected one that was part
of an action.

I'm not too happy about how specific this becomes. It's similar to Float
but in general we don't do this deep comparison.

See vercel/next.js#50087
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
…tion fields (#26846)

This solves an issue where if you inject a hidden field in the beginning
of the form, we might mistakenly hydrate the injected one that was part
of an action.

I'm not too happy about how specific this becomes. It's similar to Float
but in general we don't do this deep comparison.

See vercel/next.js#50087

DiffTrain build for commit a1f9758.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants