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 babel-plugin-react-native-web #4125

Closed

Conversation

paularmstrong
Copy link

@paularmstrong paularmstrong commented Mar 8, 2018

Fixes necolas/react-native-web#629

Problem: Current support for react-native-web uses a webpack alias. This causes the entirety of react-native-web to be bundled when building for production. Unfortunately, this means that every single module, used or not, will be bundled. This can grow app sizes by upwards of 50KiB without any benefit.

Solution: react-native-web publishes a babel plugin that handles aliasing of react-native -> react-native-web and also swaps the import paths so that it doesn't import the entire app, but just the specific modules used. Info at necolas/react-native-web/packages/babel-plugin-react-native-web

Test Plan:

  1. run yarn create-react-app test
  2. cd test && yarn add react-native-web
  3. Add import { View } from 'react-native'; to App.js and use it in place of any div
  4. yarn test
  5. yarn start
  6. yarn build
  7. yarn eject and repeat steps 4-6

NOTE: AppVeyor failure looks like a fluke. Can anyone verify?

@necolas
Copy link

necolas commented Mar 8, 2018

You might want to keep the webpack alias in there for people installing node modules, as CRA won't run Babel over everything and can't be configured to so so.

@paularmstrong
Copy link
Author

You might want to keep the webpack alias in there for people installing node modules

Can you give an example of this? I don't understand the problem. Sounds like advanced usage in which someone would want to eject.

@necolas
Copy link

necolas commented Mar 8, 2018

CRA doesn't run Babel over node modules. If someone installs a package that imports "react-native" it will throw an error unless the module is passed through Babel, or the webpack alias is in place

@iansu
Copy link
Contributor

iansu commented Mar 8, 2018

This seems reasonable to me. In the future we plan to compile node modules so we can remove the alias then. Are there any issues with using the alias and this plugin together?

@paularmstrong
Copy link
Author

paularmstrong commented Mar 8, 2018

CRA doesn't run Babel over node modules. If someone installs a package that imports "react-native" it will throw an error unless the module is passed through Babel, or the webpack alias is in place

Ah, didn't think of that case, since I haven't used any external modules that import react-native.

@iansu the babel plugin works over both react-native and react-native-web imports, so even if webpack's module aliasing went first, the babel plugin would return the same final result.

@necolas
Copy link

necolas commented Mar 8, 2018

In the future we plan to compile node modules so we can remove the alias then

IIRC, Dan said that wouldn't involve compiling all dependencies

@paularmstrong paularmstrong changed the title Use babel-plugin-react-native-web instead of webpack alias Use babel-plugin-react-native-web Mar 8, 2018
@paularmstrong paularmstrong force-pushed the parmstrong/rnw-babel-plugin branch from 49ef620 to 6a1d05f Compare March 17, 2018 23:25
@andriijas
Copy link
Contributor

Webpack 4 has built-in tree shaking rather than relying on uglifyjs which improves code elimination efficiently. I wonder if these extra kb will be shaken when we upgrade.

@necolas
Copy link

necolas commented Mar 19, 2018

Webpack 4 has built-in tree shaking

Doesn't that require an entire package to be marked as having no side effects?

@necolas
Copy link

necolas commented May 25, 2018

This can probably be closed as webpack will now tree-shake the ES modules

@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants