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

chore(babel-preset-expo, metro-config): Move majority of babel plugins to babel preset expo #25461

Merged

Conversation

EvanBacon
Copy link
Contributor

@EvanBacon EvanBacon commented Nov 18, 2023

Why

Metro's default design injects babel plugins across a number of different transformation layers and files. This makes working with Expo/React Native extremely confusing. This PR aims to move the majority of babel-related logic to babel-preset-expo. This has the following benefits:

  • Babel has a standard plugin system, Metro does not, by offloading more transformation to Babel we effectively enable a Babel-based plugin architecture for Expo Metro users.
  • By moving more of the options to Babel via the caller we can preserve the caching behavior from before but with less fragmentation as we do most config caching in Babel.
  • Having most the transformations in one place makes it easier to reason about how source must be transformed to work in Expo-supported runtimes. This makes it easier to migrate to other tools like SWC by rewriting babel-preset-expo, as opposed to by rewriting metro-transform-worker, metro-babel-transformer, and metro-babel-preset-react-native.
  • Fix ENG-10578

This PR also fixes a weird edge case where the import behavior works differently when no babel config exists. We'll do another pass to unify this logic in the future by passing the inlineRequires and transform support as caller options.

How

Consolidate our transformer.babelTransformerPath implementation and move the additional babel plugins to babel-preset-expo.
We also drop the inlineRequires transform which is already applied in the metro-transform-worker here https://github.com/facebook/metro/blob/55b45becb1fbb4237ecc5fd0d2c81f199aefe09e/packages/metro-transform-worker/src/index.js#L293-L301 -- this also matches the upstream change facebook/react-native#38771

Test Plan

  • Added new tests for the babel transforms.

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Nov 18, 2023
Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Nov 18, 2023
Copy link

linear bot commented Nov 19, 2023

@EvanBacon EvanBacon marked this pull request as ready for review November 20, 2023 19:59
@EvanBacon EvanBacon merged commit bf2d187 into main Nov 21, 2023
18 of 20 checks passed
@EvanBacon EvanBacon deleted the @evanbacon/metro-config/move-all-config-to-babel-preset-expo branch November 21, 2023 18:03
onizam95 pushed a commit to onizam95/expo-av-drm that referenced this pull request Jan 15, 2024
…s to babel preset expo (expo#25461)

# Why

Metro's default design injects babel plugins across a number of
different transformation layers and files. This makes working with
Expo/React Native extremely confusing. This PR aims to move the majority
of babel-related logic to `babel-preset-expo`. This has the following
benefits:

- Babel has a standard plugin system, Metro does not, by offloading more
transformation to Babel we effectively enable a Babel-based plugin
architecture for Expo Metro users.
- By moving more of the options to Babel via the caller we can preserve
the caching behavior from before but with less fragmentation as we do
most config caching in Babel.
- Having most the transformations in one place makes it easier to reason
about how source must be transformed to work in Expo-supported runtimes.
This makes it easier to migrate to other tools like SWC by rewriting
babel-preset-expo, as opposed to by rewriting metro-transform-worker,
metro-babel-transformer, and metro-babel-preset-react-native.
- Fix ENG-10578

This PR also fixes a weird edge case where the import behavior works
differently when no babel config exists. We'll do another pass to unify
this logic in the future by passing the inlineRequires and transform
support as caller options.

<!--
Please describe the motivation for this PR, and link to relevant GitHub
issues, forums posts, or feature requests.
-->

# How

Consolidate our `transformer.babelTransformerPath` implementation and
move the additional babel plugins to `babel-preset-expo`.
We also drop the inlineRequires transform which is already applied in
the `metro-transform-worker` here
https://github.com/facebook/metro/blob/55b45becb1fbb4237ecc5fd0d2c81f199aefe09e/packages/metro-transform-worker/src/index.js#L293-L301
-- this also matches the upstream change
facebook/react-native#38771

# Test Plan

- Added new tests for the babel transforms.

---------

Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint changed bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants