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

config ignores peerDependencies and instead reads devDependencies #2419

Closed
tido64 opened this issue Jun 20, 2024 · 3 comments · Fixed by #2423
Closed

config ignores peerDependencies and instead reads devDependencies #2419

tido64 opened this issue Jun 20, 2024 · 3 comments · Fixed by #2423

Comments

@tido64
Copy link
Contributor

tido64 commented Jun 20, 2024

Description

config ignores peerDependencies and instead reads devDependencies:

const deps = [
...Object.keys(pjson.dependencies || {}),
...Object.keys(pjson.devDependencies || {}),
];

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
Copy link
Contributor Author

tido64 commented Jun 20, 2024

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

@szymonrybczak
Copy link
Collaborator

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 😕

Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants