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

Fix resolving node modules directory with custom exports #1655

Merged

Conversation

fortmarek
Copy link
Contributor

@fortmarek fortmarek commented Jul 25, 2022

Summary:

Fixes #1168

As described in the issue, it's currently not possible to use RN CLI with packages that have exports in their package.json and do not contain package.json itself in the exports object. For example, d3 dependency does just that. While maintainers could make their package compatible with the RN CLI, such an effort would be huge and not all maintainers want to do that – understandably so, especially if their packages are not exclusively for RN.

To fix this, I've imported this method from the @rnx-kit/tools-node package that does not depend on the require.resolve method (which currently does not work in the case described above). Alternatively, we could just import the method itself but the package is quite small, so in this case, it's probably better in the long run to depend on it instead.

Test Plan:

  • Clone this sample
  • cd into it and run yarn
  • Run yarn start
  • Observe error similar to:
error Failed to load configuration of your project.
Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './package.json' is not defined by "exports" in /Users/marekfort/Downloads/RN0692/node_modules/d3/package.json
    at new NodeError (node:internal/errors:387:5)
    at throwExportsNotFound (node:internal/modules/esm/resolve:439:9)
    at packageExportsResolve (node:internal/modules/esm/resolve:718:3)
    at resolveExports (node:internal/modules/cjs/loader:493:36)
    at Module._findPath (node:internal/modules/cjs/loader:533:31)
    at Module._resolveFilename (node:internal/modules/cjs/loader:942:27)
    at Function.resolve (node:internal/modules/cjs/helpers:108:19)
    at resolveNodeModuleDir (/Users/marekfort/src/github.com/fortmarek/cli/packages/cli-tools/build/resolveNodeModuleDir.js:24:42)
    at /Users/marekfort/src/github.com/fortmarek/cli/packages/cli-config/build/loadConfig.js:93:76
    at Array.reduce (<anonymous>)
info Run CLI with --verbose flag for more details.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
  • Clone the branch from this PR
  • Set up CLI as per these instructions
  • Once the custom CLI is linked inside the repro sample, run yarn start again
  • No error should occur

Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I decided to vendor the implementation of pickValues and findPackageDependencyDir due to the @rnx-kit/tools-node package adding roughly 42kB: https://packagephobia.com/result?p=%40rnx-kit%2Ftools-node. RN CLI already downloads a lot of stuff and we need to make a better job at keeping it lean so that npx react-native init speed is good enough.

@thymikee thymikee merged commit a97c026 into react-native-community:main Jul 25, 2022
thymikee added a commit that referenced this pull request Jul 25, 2022
* Fix resolving node modules directory with custom exports

* vendor rnx-kit helpers to reduce node_modules size

Co-authored-by: Michał Pierzchała <thymikee@gmail.com>
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Jul 27, 2022
Summary:
Bumping main to latest version of CLI available; in particular, we want to ensure to propagate this fix react-native-community/cli#1655.

After merging, we'll cherry-pick in the `0.70-stable` branch.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[General] [Changed] - bump CLI to v9.0.0-alpha.5

Pull Request resolved: #34275

Test Plan: CI

Reviewed By: NickGerleman

Differential Revision: D38151383

Pulled By: dmitryrykun

fbshipit-source-id: afdafb496a159ad766dd322a4aab4bda6146edf0
kelset added a commit to facebook/react-native that referenced this pull request Jul 27, 2022
Summary:
Bumping main to latest version of CLI available; in particular, we want to ensure to propagate this fix react-native-community/cli#1655.

After merging, we'll cherry-pick in the `0.70-stable` branch.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[General] [Changed] - bump CLI to v9.0.0-alpha.5

Pull Request resolved: #34275

Test Plan: CI

Reviewed By: NickGerleman

Differential Revision: D38151383

Pulled By: dmitryrykun

fbshipit-source-id: afdafb496a159ad766dd322a4aab4bda6146edf0

# Conflicts:
#	yarn.lock
roryabraham pushed a commit to Expensify/react-native that referenced this pull request Aug 17, 2022
Summary:
Bumping main to latest version of CLI available; in particular, we want to ensure to propagate this fix react-native-community/cli#1655.

After merging, we'll cherry-pick in the `0.70-stable` branch.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[General] [Changed] - bump CLI to v9.0.0-alpha.5

Pull Request resolved: facebook#34275

Test Plan: CI

Reviewed By: NickGerleman

Differential Revision: D38151383

Pulled By: dmitryrykun

fbshipit-source-id: afdafb496a159ad766dd322a4aab4bda6146edf0
roryabraham pushed a commit to Expensify/react-native that referenced this pull request Aug 17, 2022
Summary:
Bumping main to latest version of CLI available; in particular, we want to ensure to propagate this fix react-native-community/cli#1655.

After merging, we'll cherry-pick in the `0.70-stable` branch.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[General] [Changed] - bump CLI to v9.0.0-alpha.5

Pull Request resolved: facebook#34275

Test Plan: CI

Reviewed By: NickGerleman

Differential Revision: D38151383

Pulled By: dmitryrykun

fbshipit-source-id: afdafb496a159ad766dd322a4aab4bda6146edf0
@hocuf
Copy link

hocuf commented Jan 7, 2023

i did but it doesnt work. omg

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

Successfully merging this pull request may close these issues.

Package subpath './package.json' is not defined by "exports"
3 participants