Skip to content

Enable moveBefore in experimental releases #32549

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

Merged
merged 1 commit into from
Mar 10, 2025

Conversation

sebmarkbage
Copy link
Collaborator

Enabling feature detection of early DOM features in a framework is reckless. I'm not judging other frameworks (but also a little bit). Because if you do something like if (moveBefore) moveBefore(a, b) else insertBefore(a, b) like we do and then the implementation has to change there are still too many websites out there that it becomes impossible to change it. It would break the web. It would instead have to change to a different name. That's what happened with contains -> includes. Counter to popular belief it didn't have anything to do with patching prototypes. Therefore, ideally frameworks shouldn't start rely on it until there's two implementations so that there's time for feedback.

That's why we didn't immediately enable this even in experimental. However, at this point there's probably enough feature detection and it has shipped long enough in Chrome that it's unlikely to be able to change at this point.

We can enable it now. For now just in @experimental to see if we can flush out issues with it before bringing it to stable.

@react-sizebot
Copy link

Comparing: f9d7808...f449938

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.js = 6.68 kB 6.68 kB = 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 518.54 kB 518.54 kB = 92.45 kB 92.45 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js +0.10% 589.31 kB 589.90 kB +0.12% 104.91 kB 105.03 kB
facebook-www/ReactDOM-prod.classic.js = 642.76 kB 642.76 kB = 113.01 kB 113.01 kB
facebook-www/ReactDOM-prod.modern.js = 633.08 kB 633.08 kB = 111.44 kB 111.44 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against e5ebdd6

@sebmarkbage sebmarkbage merged commit 696950a into facebook:main Mar 10, 2025
196 checks passed
github-actions bot pushed a commit that referenced this pull request Mar 10, 2025
Enabling feature detection of early DOM features in a framework is
reckless. I'm not judging other frameworks (but also a little bit).
Because if you do something like `if (moveBefore) moveBefore(a, b) else
insertBefore(a, b)` like we do and then the implementation has to change
there are still too many websites out there that it becomes impossible
to change it. It would break the web. It would instead have to change to
a different name. That's what happened with `contains` -> `includes`.
Counter to popular belief it didn't have anything to do with patching
prototypes. Therefore, ideally frameworks shouldn't start rely on it
until there's two implementations so that there's time for feedback.

That's why we didn't immediately enable this even in experimental.
However, at this point there's probably enough feature detection and it
has shipped long enough in Chrome that it's unlikely to be able to
change at this point.

We can enable it now. For now just in `@experimental` to see if we can
flush out issues with it before bringing it to stable.

DiffTrain build for [696950a](696950a)
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