facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
119.25k stars 24.34k forks source link

False positive metro config warning #43182

Closed paulschreiber closed 1 month ago

paulschreiber commented 8 months ago

Description

When I start my app locally (npm run ios), I get this error:

warn =================================================================================================
warn From React Native 0.73, your project's Metro config should extend '@react-native/metro-config'
warn or it will fail to build. Please copy the template at:
warn https://github.com/facebook/react-native/blob/main/packages/react-native/template/metro.config.js
warn This warning will be removed in future (https://github.com/facebook/metro/issues/1018).
warn =================================================================================================

My metro.config.js looks like this:

const { getDefaultConfig } = require('expo/metro-config');
const { mergeConfig } = require('@react-native/metro-config');

/**
 * Metro configuration
 * https://facebook.github.io/metro/docs/configuration
 *
 * @type {import('metro-config').MetroConfig}
 */
const config = {};

module.exports = mergeConfig(getDefaultConfig(__dirname), config);

Since I am using Expo, I need to extend that, too.

Steps to reproduce

  1. Create a new React Native project
  2. npx install-expo-modules@latest
  3. npm run ios

React Native Version

0.73.4

Affected Platforms

Build - MacOS

Output of npx react-native info

System:
  OS: macOS 14.3.1
  CPU: (10) arm64 Apple M1 Pro
  Memory: 3.58 GB / 32.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 20.9.0
    path: ~/.nvm/versions/node/v20.9.0/bin/node
  Yarn:
    version: 1.22.21
    path: /opt/homebrew/bin/yarn
  npm:
    version: 10.2.5
    path: ~/.nvm/versions/node/v20.9.0/bin/npm
  Watchman:
    version: 2024.01.22.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.15.2
    path: /Users/paul/.rbenv/shims/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.2
      - iOS 17.2
      - macOS 14.2
      - tvOS 17.2
      - visionOS 1.0
      - watchOS 10.2
  Android SDK:
    API Levels:
      - "27"
      - "29"
      - "30"
      - "33"
      - "34"
    Build Tools:
      - 30.0.3
      - 33.0.0
      - 33.0.1
      - 34.0.0
    System Images:
      - android-33 | Google APIs ARM 64 v8a
    Android NDK: Not Found
IDEs:
  Android Studio: 2022.3 AI-223.8836.35.2231.10811636
  Xcode:
    version: 15.2/15C500b
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.9
    path: /opt/homebrew/bin/javac
  Ruby:
    version: 3.2.2
    path: /Users/paul/.rbenv/shims/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.73.4
    wanted: ^0.73.4
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: true
  newArchEnabled: false

Stacktrace or Logs

warn =================================================================================================
warn From React Native 0.73, your project's Metro config should extend '@react-native/metro-config'
warn or it will fail to build. Please copy the template at:
warn https://github.com/facebook/react-native/blob/main/packages/react-native/template/metro.config.js
warn This warning will be removed in future (https://github.com/facebook/metro/issues/1018).
warn =================================================================================================

Reproducer

https://github.com/paulschreiber/metro-config-warning

Screenshots and Videos

No response

cortinico commented 8 months ago

Yup this is a valid report as the code to fire this warning lives here: https://github.com/facebook/react-native/blob/87692454774d797cd3d7f26a0c31d53a4c90cfdd/packages/community-cli-plugin/src/utils/loadMetroConfig.js#L83-L109

@huntie do you know what might be going on here?

huntie commented 8 months ago

Interesting. @paulschreiber Your Metro config extends expo/metro-config (rather than @react-native/metro-config), and is being consumed via React Native Community CLI (rather than Expo CLI which would usually be the target for expo/metro-config). This will be the cause.

Resolution: I'd suggest copying the template package.json exactly, and then adding back either 1/ parts of expo/metro-config as necessary, or 2/ all of expo/metro-config and placing it 2nd into the mergeConfig call (shown).

- const { getDefaultConfig } = require('expo/metro-config');
+ const { getDefaultConfig: getExpoDefaultConfig } = require('expo/metro-config');
- const { mergeConfig } = require('@react-native/metro-config');
+ const { getDefaultConfig, mergeConfig } = require('@react-native/metro-config');

/**
 * Metro configuration
 * https://facebook.github.io/metro/docs/configuration
 *
 * @type {import('metro-config').MetroConfig}
 */
const config = {};

- module.exports = mergeConfig(getDefaultConfig(__dirname), config);
+ module.exports = mergeConfig(getDefaultConfig(__dirname), getExpoDefaultConfig(__dirname), config);

As for whether the warning should happen in this case, if extending expo/metro-config only — it's correct, but not ideal. We designed the check to be robust by settting a global __REACT_NATIVE_METRO_CONFIG_LOADED variable, specific to @react-native/metro-config, with the expectation that this should be loaded in every user's config that inputs to CLI.

The warning will also be removed in future once we are confident people have migrated, likely 0.75.

cc @byCedric — Relevant to decision around expo/metro-config not directly extending @react-native/metro-config itself. We can review this, but it's not necessarily the direction we have to go. The above user fix is the valid approach.

paulschreiber commented 8 months ago

@huntie As noted in steps to reproduce, this config file was created by the React Native and Expo scaffolding.

It's unclear from your comment who should make the changes to metro.config.js (I assume package.json is a typo).

huntie commented 8 months ago

@paulschreiber Right, this is part of Expo's instructions with npx install-expo-modules@latest, I see now 👍🏻.

image

https://docs.expo.dev/bare/installing-expo-modules/#optional-use-expo-cli-for-bundling

I'll follow up with @byCedric.

iamnynvk commented 7 months ago

Where is the React native CLI solution ?

wilkinson4 commented 2 months ago

Just going to put this here because it gave me a big headache when trying to integrate expo-doctor into our github ci actions. We had an issue with bundling/transforming where we had some redux actions being imported like so:

import * as actions from './someActionsFile'

and using redux's connect function we were using those actions to connect to props like so:

export default connect(mapStateToProps, {someAction: actions.someAction}, someComponent)

For some reason when we used only getDefaultConfig from expo/metro-config the actions.someAction was undefined at call time on initial load, BUT if you saved the file to trigger a HMR it was now defined as expected. After changing our metro.config.js to look like this it fixed the problem:

const { getDefaultConfig: getExpoDefaultConfig } = require('expo/metro-config');
const { getDefaultConfig, mergeConfig } = require('@react-native/metro-config');

/**
 * Metro configuration
 * https://facebook.github.io/metro/docs/configuration
 *
 * @type {import('metro-config').MetroConfig}
 */
const config = {
  transformer: {
    assetPlugins: ['expo-asset/tools/hashAssetFiles']
  }
};

module.exports = mergeConfig(
  getExpoDefaultConfig(__dirname),
  getDefaultConfig(__dirname),
  config
);

This was in a bare react-native project that was around before expo became so popular so when we added expo I could've missed something in the documentation, but this was definitely a tricky bug.

paulschreiber commented 1 month ago

@huntie which PR resolved the issue?

huntie commented 1 month ago

@paulschreiber As far as I can see, this is a user-space config issue. There are now workaround snippets above demonstrating the correct mergeConfig expression.

wilkinson4 commented 1 month ago

Just going to put this here because it gave me a big headache when trying to integrate expo-doctor into our github ci actions. We had an issue with bundling/transforming where we had some redux actions being imported like so:

import * as actions from './someActionsFile'

and using redux's connect function we were using those actions to connect to props like so:

export default connect(mapStateToProps, {someAction: actions.someAction}, someComponent)

For some reason when we used only getDefaultConfig from expo/metro-config the actions.someAction was undefined at call time on initial load, BUT if you saved the file to trigger a HMR it was now defined as expected. After changing our metro.config.js to look like this it fixed the problem:

const { getDefaultConfig: getExpoDefaultConfig } = require('expo/metro-config');
const { getDefaultConfig, mergeConfig } = require('@react-native/metro-config');

/**
 * Metro configuration
 * https://facebook.github.io/metro/docs/configuration
 *
 * @type {import('metro-config').MetroConfig}
 */
const config = {
  transformer: {
    assetPlugins: ['expo-asset/tools/hashAssetFiles']
  }
};

module.exports = mergeConfig(
  getExpoDefaultConfig(__dirname),
  getDefaultConfig(__dirname),
  config
);

This was in a bare react-native project that was around before expo became so popular so when we added expo I could've missed something in the documentation, but this was definitely a tricky bug.

Quick update. We ended up switching our metro config to this once we upgraded to expo 51 and RN 0.74.5:

const { getDefaultConfig: getExpoDefaultConfig } = require('expo/metro-config');
const { getDefaultConfig, mergeConfig } = require('@react-native/metro-config');

/**
 * Metro configuration
 * https://facebook.github.io/metro/docs/configuration
 *
 * @type {import('metro-config').MetroConfig}
 */

const expoConfig = getExpoDefaultConfig(__dirname);
const defaultConfig = getDefaultConfig(__dirname);

const config = {
  transformer: {
    ...expoConfig.transformer,
    ...defaultConfig.transformer,
    assetPlugins: ['expo-asset/tools/hashAssetFiles']
  }
};

module.exports = mergeConfig(defaultConfig, expoConfig, config);
kaer23 commented 1 week ago

The warning you're encountering suggests that your Metro config should extend @react-native/metro-config to avoid build failures. To ensure compatibility, update your metro.config.js as per the React Native template. For more on optimizing configurations for projects like topcelebrite.fr, follow the React Native setup guidelines closely.