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

Missing dependency on preact / react (ghost dependency) #262

Open
blutorange opened this issue Feb 3, 2025 · 3 comments · May be fixed by #263
Open

Missing dependency on preact / react (ghost dependency) #262

blutorange opened this issue Feb 3, 2025 · 3 comments · May be fixed by #263

Comments

@blutorange
Copy link

blutorange commented Feb 3, 2025

This module tries to import / require from preact, but does not declare it as a dependency in its package.json (a ghost dependency). This is an error, yarn requires all dependency to be listed properly:

Module not found: Error: Can't resolve 'preact' in 'C:\Users\user\AppData\Local\Yarn\Berry\cache\htm-npm-3.1.1-e3b831f850-10.zip\node_modules\htm\preact'
resolve 'preact' in 'C:\Users\user\AppData\Local\Yarn\Berry\cache\htm-npm-3.1.1-e3b831f850-10.zip\node_modules\htm\preact'
  Parsed request is a module
  using description file: C:\Users\user\AppData\Local\Yarn\Berry\cache\htm-npm-3.1.1-e3b831f850-10.zip\node_modules\htm\preact\package.json (relative path: .)
    Field 'browser' doesn't contain a valid alias configuration
    resolve as module
      request is not managed by the pnpapi
        htm tried to access preact, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.
        Required package: preact
        Required by: htm@npm:3.1.1 (via C:\Users\user\AppData\Local\Yarn\Berry\cache\htm-npm-3.1.1-e3b831f850-10.zip\node_modules\htm\preact\)
 @ ../../../../../../AppData/Local/Yarn/Berry/cache/@bpmn-io-diagram-js-ui-npm-0.2.3-01795dd8e0-10.zip/node_modules/@bpmn-io/diagram-js-ui/lib/index.js 1:0-34 1:0-34
 @ ../../../../../../AppData/Local/Yarn/Berry/cache/diagram-js-npm-15.2.4-de7db86f90-10.zip/node_modules/diagram-js/lib/ui/index.js 1:0-39 1:0-39
 @ ../../../../../../AppData/Local/Yarn/Berry/cache/diagram-js-npm-15.2.4-de7db86f90-10.zip/node_modules/diagram-js/lib/features/popup-menu/PopupMenu.js 1:0-4:18 132:2-8 133:4-8 278:2-8  
 @ ../../../../../../AppData/Local/Yarn/Berry/cache/diagram-js-npm-15.2.4-de7db86f90-10.zip/node_modules/diagram-js/lib/features/popup-menu/index.js 1:0-36 12:23-32
 @ ../../../../../../AppData/Local/Yarn/Berry/cache/bpmn-js-npm-18.2.0-5474b88486-10.zip/node_modules/bpmn-js/lib/features/align-elements/index.js 3:0-65 13:4-19
 @ ../../../../../../AppData/Local/Yarn/Berry/cache/bpmn-js-npm-18.2.0-5474b88486-10.zip/node_modules/bpmn-js/lib/Modeler.js 12:0-60 167:2-21
 @ ./src/bpmn-editor/render-bpmn-editor.ts 7:0-48 33:31-41
 @ ./src/bpmn-editor/index.ts 1:0-40 1:0-40

The same goes for react. Possible ways how the dependencies could be fixed:

  • declare both react and preact as dependencies (will always install both preact and react)
  • declare both react and preact as optional peer dependencies (preferred) (will force users to install both react and preact)
  • create 2 separate NPM packages, so that each package can declare its dependencies properly

Workaround

For people affected: You can add the following bandage aid to fix the declared dependencies (file .yarnrc.yml):

packageExtensions:
  htm@*:
    peerDependencies:
      preact: "*"
      react: "*"
    peerDependenciesMeta:
    preact:
      optional: true
    react:
      optional: true
@rschristian
Copy link

declare both react and preact as optional peer dependencies (will force users to install both react and preact)

Why would optional peer deps on both force users to install both? If they're both optional then they can be installed/used independently in other package managers just fine.

@blutorange
Copy link
Author

@rschristian Thinking about it again, you're actually right. So marking them as optional peer dependencies should be way to go.

@rschristian
Copy link

Feel free to make a PR (or I can do at some point). Not a maintainer here unfortunately so I can't actually get it in, but w/ a PR made, it's at least available for easy merging in the future.

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 a pull request may close this issue.

2 participants