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

Use require() to implement script src in tests #26717

Merged

Conversation

sebmarkbage
Copy link
Collaborator

We currently use rollup to make an adhoc bundle from the file system when we're testing an import of an external file.

This doesn't follow all the interception rules that we use in jest and in our actual builds.

This switches to just using jest require() to load these. This means that they effectively have to load into the global document so this only works with global document tests which is all we have now anyway.

@sebmarkbage sebmarkbage requested review from gnoff and mofeiZ April 24, 2023 20:08
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Apr 24, 2023
@react-sizebot
Copy link

react-sizebot commented Apr 24, 2023

Comparing: 5e5342b...a65bfdc

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 = 163.92 kB 163.92 kB = 51.68 kB 51.67 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 168.70 kB 168.70 kB = 53.04 kB 53.04 kB
facebook-www/ReactDOM-prod.classic.js = 570.27 kB 570.27 kB = 100.93 kB 100.93 kB
facebook-www/ReactDOM-prod.modern.js = 554.00 kB 554.00 kB = 98.26 kB 98.26 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against a65bfdc

We currently use rollup to make an adhoc bundle from the file system when
we're testing an import of an external file.

This doesn't follow all the interception rules that we use in jest and in
our actual builds.

This switches to just using jest require() to load these. This means that
they effectively have to load into the global document so this only works
with global document tests which is all we have now anyway.
@sebmarkbage sebmarkbage force-pushed the userequireforscriptsrcinjection branch from 97e8ddb to a65bfdc Compare April 24, 2023 20:12
@sebmarkbage
Copy link
Collaborator Author

This is blocking #26716 since I'm using a feature flag in the external runtime there. I could potentially remove the feature flag since it's probably not that much damage but seems like the right thing to keep it.

@sebmarkbage sebmarkbage merged commit 64d6be7 into facebook:main Apr 25, 2023
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
We currently use rollup to make an adhoc bundle from the file system
when we're testing an import of an external file.

This doesn't follow all the interception rules that we use in jest and
in our actual builds.

This switches to just using jest require() to load these. This means
that they effectively have to load into the global document so this only
works with global document tests which is all we have now anyway.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
We currently use rollup to make an adhoc bundle from the file system
when we're testing an import of an external file.

This doesn't follow all the interception rules that we use in jest and
in our actual builds.

This switches to just using jest require() to load these. This means
that they effectively have to load into the global document so this only
works with global document tests which is all we have now anyway.

DiffTrain build for commit 64d6be7.
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