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

Prevent bundling of Node polyfills when importing TestUtils/TestRenderer #15305

Merged
merged 1 commit into from
Apr 3, 2019

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Apr 3, 2019

I think this fixes #14853 (comment).

In particular, there are two cases we'd like to avoid:

  • We don't want Webpack to emit a warning about require expression being dynamic.
  • We don't want Webpack to include its Node polyfills (for example, for timers or for global) because we access them. (This is happening on master.)

Effectively, we want to obscure the require call from webpack. There are a few webpack-specific ways to do it but they seem brittle. Additionally, naïve ways to obscure it (like the current code) are inlined by GCC at our build step for the prod bundle.

Instead, I'm reading module.require — which doesn't even exist in webpack runtime but exists in Node. Webpack doesn't analyze it. To further strengthen it, I make the require string itself dynamic. Seems like this is sufficient. I only tested webpack and Node. It might be worth testing that browserify or Rollup work as expected — but they also not known for noisy warnings so I assume it should be fine.

This wouldn't work if TestRender or TestUtils were a Node ESM because there would be no require in it. But that's status quo anyway because our current require call wouldn't work either.

@gaearon gaearon merged commit e5c5935 into facebook:master Apr 3, 2019
@gaearon gaearon deleted the no-polyfill branch April 3, 2019 15:12
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.

3 participants