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

feat(metro-config)!: Enable inlineRequires by default #25089

Merged
merged 10 commits into from Nov 2, 2023

Conversation

EvanBacon
Copy link
Contributor

@EvanBacon EvanBacon commented Oct 27, 2023

Why

Align with upstream defaults more closely. This feature may cause unexpected behavior with global side-effects so I'm marking it as a breaking change. Users can disable the feature:

const { getDefaultConfig } = require('expo/metro-config');

const config = getDefaultConfig(__dirname);

config.transformer.getTransformOptions = async () => ({
  transform: {
    experimentalImportSupport: false,
    inlineRequires: false,
  },
});

module.exports = config;

Test Plan

  • This feature isn't harmful unless the order of esm transpilation is shifted. Current tests should cover the updates here.

Checklist

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Oct 27, 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 Oct 31, 2023
@expo-bot expo-bot added bot: suggestions ExpoBot has some suggestions and removed bot: passed checks ExpoBot has nothing to complain about labels Oct 31, 2023
@EvanBacon EvanBacon marked this pull request as ready for review November 2, 2023 18:29
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Nov 2, 2023
@expo-bot expo-bot added bot: needs changes ExpoBot found things that don't meet our guidelines and removed bot: passed checks ExpoBot has nothing to complain about labels Nov 2, 2023
@expo-bot expo-bot added bot: suggestions ExpoBot has some suggestions and removed bot: needs changes ExpoBot found things that don't meet our guidelines labels Nov 2, 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 2, 2023
@EvanBacon EvanBacon merged commit 7a36f8a into main Nov 2, 2023
13 of 14 checks passed
@EvanBacon EvanBacon deleted the @evanbacon/metro-config/enable-inlineRequires branch November 2, 2023 19:31
EvanBacon added a commit that referenced this pull request Dec 1, 2023
# Why

This PR reverts #25089 where we enabled
inlineRequires by default.

The inline requires feature doesn't work as-is because it does not
respect module side-effects. Due to the way inline requires is computed
in the transformer, it has no ability to determine if a module should be
automatically skipped to prevent breaking the expected order of
instantiation.

---------

Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
onizam95 pushed a commit to onizam95/expo-av-drm that referenced this pull request Jan 15, 2024
# Why

This PR reverts expo#25089 where we enabled
inlineRequires by default.

The inline requires feature doesn't work as-is because it does not
respect module side-effects. Due to the way inline requires is computed
in the transformer, it has no ability to determine if a module should be
automatically skipped to prevent breaking the expected order of
instantiation.

---------

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