react-native-community / cli

The React Native Community CLI - command line tools to help you build RN apps
MIT License
2.35k stars 900 forks source link

`config` ignores `peerDependencies` and instead reads `devDependencies` #2419

Closed tido64 closed 1 week ago

tido64 commented 3 months ago

Description

config ignores peerDependencies and instead reads devDependencies:

https://github.com/react-native-community/cli/blob/33646ef94a8cb4f9ce10d6db336c7a124144025b/packages/cli-config/src/findDependencies.ts#L18-L21

This leads to errors in react-native-windows autolinking (and possibly in other scenarios) where dependencies are only declared under peerDependencies and not devDependencies, for instance in @react-native-webapis/web-storage we have the following:

{
  "peerDependencies": {
    "@callstack/react-native-visionos": ">=0.73",
    "react": ">=18.2.0",
    "react-native": ">=0.72",
    "react-native-macos": ">=0.72",
    "react-native-windows": ">=0.72"
  },
  "peerDependenciesMeta": {
    "@callstack/react-native-visionos": {
      "optional": true
    },
    "react-native-macos": {
      "optional": true
    },
    "react-native-windows": {
      "optional": true
    }
  },
  "devDependencies": {
    "react-native": "^0.73.0"
  }
}

In this case, config outputs:

  "platforms": {
    "ios": {},
    "android": {}
  },

If I add react-native-windows to devDependencies, config outputs:

  "platforms": {
    "ios": {},
    "android": {},
    "windows": {
      "npmPackageName": "react-native-windows"
    }
  },

In my opinion, the correct fix should be to replace devDependencies with peerDependencies, but I don't know if this will break current scenarios so we should probably keep it as is and just add peerDependencies to the list in findDependencies.ts.

Reproducible Demo

In any project, you can run the following:

yarn add @react-native-webapis/web-storage@0.2.6
cd node_modules/@react-native-webapis/web-storage
yarn react-native config
tido64 commented 3 months ago

@thymikee: Let me know what you think. I can submit a fix once we agree on a solution.

szymonrybczak commented 3 months ago

Ideally we should rely on peerDependencies not on devDependencies and I'm totally fine with adding peerDependencies to the places that we check. But removing devDependencies sounds like a breaking change 😕

github-actions[bot] commented 1 week ago

There hasn't been any activity on this issue in the past 3 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.