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

@wordpress/primitives is missing a dependency on react #62250

Closed
anomiex opened this issue Jun 3, 2024 · 3 comments · Fixed by #64218
Closed

@wordpress/primitives is missing a dependency on react #62250

anomiex opened this issue Jun 3, 2024 · 3 comments · Fixed by #64218
Assignees
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time npm Packages Related to npm packages [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@anomiex
Copy link
Contributor

anomiex commented Jun 3, 2024

@wordpress/primitives is lacking a dependency or peer dependency on react.

This happens to work with npm's hoisting due to other dependencies pulling that package in, but will fail with yarn's p'n'p or pnpm with hoisting disabled.

This seems to have begun with #61692.

Reproduction

With yarn:

  1. Create a temporary directory, and cd into it.
  2. echo '{}' > package.json
  3. yarn set version stable
  4. yarn add @wordpress/primitives
  5. Run yarn node -e 'const x = require( "@wordpress/primitives" );'

With pnpm:

  1. Create a temporary directory, and cd into it.
  2. echo 'hoist-pattern=[]' > .npmrc
  3. pnpm add @wordpress/primitives (note pnpm 8 defaults to installing peer deps)
  4. Run node -e 'const x = require( "@wordpress/primitives" );'

Expected behavior

Command runs.

Actual behavior

With yarn:

Error: @wordpress/primitives tried to access react, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: react (via "react/jsx-runtime")
Required by: @wordpress/primitives@npm:4.0.0 (via /home/user/.yarn/berry/cache/@wordpress-primitives-npm-4.0.0-bfa939d68d-10c0.zip/node_modules/@wordpress/primitives/build/svg/)

Require stack:
- /home/user/.yarn/berry/cache/@wordpress-primitives-npm-4.0.0-bfa939d68d-10c0.zip/node_modules/@wordpress/primitives/build/svg/index.js
- /home/user/.yarn/berry/cache/@wordpress-primitives-npm-4.0.0-bfa939d68d-10c0.zip/node_modules/@wordpress/primitives/build/index.js
- /tmp/test/[eval]
    at require$$0.Module._resolveFilename (/tmp/test/.pnp.cjs:5959:13)
    at Module._load (node:internal/modules/cjs/loader:986:27)
    at require$$0.Module._load (/tmp/test/.pnp.cjs:5850:31)
    at Module.require (node:internal/modules/cjs/loader:1233:19)
    at require (node:internal/modules/helpers:179:18)
    at Object.<anonymous> (/home/user/.yarn/berry/cache/@wordpress-primitives-npm-4.0.0-bfa939d68d-10c0.zip/node_modules/@wordpress/primitives/build/svg/index.js:10:19)
    at Module._compile (node:internal/modules/cjs/loader:1358:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1416:10)
    at require$$0.Module._extensions..js (/tmp/test/.pnp.cjs:6002:33)
    at Module.load (node:internal/modules/cjs/loader:1208:32)

With pnpm:

Error: Cannot find module 'react/jsx-runtime'
Require stack:
- /tmp/test/node_modules/.pnpm/@wordpress+primitives@4.0.0/node_modules/@wordpress/primitives/build/svg/index.js
- /tmp/test/node_modules/.pnpm/@wordpress+primitives@4.0.0/node_modules/@wordpress/primitives/build/index.js
- /tmp/test/[eval]
    at Module._resolveFilename (node:internal/modules/cjs/loader:1145:15)
    at Module._load (node:internal/modules/cjs/loader:986:27)
    at Module.require (node:internal/modules/cjs/loader:1233:19)
    at require (node:internal/modules/helpers:179:18)
    at Object.<anonymous> (/tmp/test/node_modules/.pnpm/@wordpress+primitives@4.0.0/node_modules/@wordpress/primitives/build/svg/index.js:10:19)
    at Module._compile (node:internal/modules/cjs/loader:1358:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1416:10)
    at Module.load (node:internal/modules/cjs/loader:1208:32)
    at Module._load (node:internal/modules/cjs/loader:1024:12)
    at Module.require (node:internal/modules/cjs/loader:1233:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/tmp/test/node_modules/.pnpm/@wordpress+primitives@4.0.0/node_modules/@wordpress/primitives/build/svg/index.js',
    '/tmp/test/node_modules/.pnpm/@wordpress+primitives@4.0.0/node_modules/@wordpress/primitives/build/index.js',
    '/tmp/test/[eval]'
  ]
}
@anomiex
Copy link
Contributor Author

anomiex commented Jun 3, 2024

cc @youknowriad @Mamaduka

@youknowriad
Copy link
Contributor

Good point, I think the root of the issue predates the PR. Basically, it seems that all packages that use JSX should have a direct react dependency. primitives is one of them, maybe there are more.

@youknowriad youknowriad added [Type] Bug An existing feature does not function as intended npm Packages Related to npm packages labels Jun 4, 2024
@gziolo gziolo added the Good First Issue An issue that's suitable for someone looking to contribute for the first time label Jun 13, 2024
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Aug 3, 2024
@Mamaduka
Copy link
Member

Mamaduka commented Aug 3, 2024

Created PR - #64218.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time npm Packages Related to npm packages [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants