-
Notifications
You must be signed in to change notification settings - Fork 46.7k
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
Break up require/import statements in strings #18222
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 49e68a0:
|
Details of bundled changes.Comparing: 6a0efdd...49e68a0 react-test-renderer
react-reconciler
react-dom
react-native-renderer
ReactDOM: size: 0.0%, gzip: 0.0% Size changes (stable) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. Add a comment to each? “Intentionally broken down require” or something. Otherwise someone will regress.
Details of bundled changes.Comparing: 6a0efdd...49e68a0 react-dom
react-test-renderer
react-reconciler
react-native-renderer
ReactDOM: size: 0.0%, gzip: 🔺+0.1% Size changes (experimental) |
FB: The stack of D20282825 explains the reasons behind this change for FB employees. |
A comment definitely, would be extra nice to have a test or lint rule. Otherwise this absolutely will regress. |
Break up require/import statements in strings (facebook#18222)
Summary: Similar to the previous diff in this stack, I'm adjusting the React files we use to make sure require and import statements don't get extracted by Jest. Right now Jest extracts them, and when it looks up dependencies in reverse it just ignores the ones that don't exist. Corresponding React PR (to make sure this change doesn't get reverted during the next sync): facebook/react#18222 Changelog: [Internal] Reviewed By: motiz88 Differential Revision: D20282829 fbshipit-source-id: 5ff6a64d31672dd29243d020e8174797a44d9267
Summary: Similar to the previous diff in this stack, I'm adjusting the React files we use to make sure require and import statements don't get extracted by Jest. Right now Jest extracts them, and when it looks up dependencies in reverse it just ignores the ones that don't exist. Corresponding React PR (to make sure this change doesn't get reverted during the next sync): facebook/react#18222 Changelog: [Internal] Reviewed By: motiz88 Differential Revision: D20282829 fbshipit-source-id: 5ff6a64d31672dd29243d020e8174797a44d9267
Summary
We have a number of dependency extractors in JavaScript tools, like Jest but also in internal FB tooling, that are using RegExp to extract static dependencies and create a graph. I am trying to reduce the amount of extractors we have at FB and noticed the Jest one has a few issues that are hard to fix with regex. I realize that RegExp dependency tracking is inherently imprecise but hope we can bear some minor inconvenience in just a few places.
In this PR I'm adjusting the React files we use to make sure require and import statements don't get extracted by Jest. Right now Jest extracts them, and when it looks up dependencies in reverse it just ignores the ones that don't exist. I'd like to make that fail hard in some tools and this change is a pre-requisite for that.
Test Plan
When running this dependencyExtractor from Jest (via jest-haste-map) on react-native, the React files (ReactFabric-dev.fb.js etc.) will no longer have invalid dependencies associated with them.