search

iTwin / itwinjs-core

Monorepo for iTwin.js Library
https://www.itwinjs.org
MIT License
620 stars 210 forks source link

Workspace peerDependencies should be specified without `^` #7373

Open saskliutas opened 5 days ago

saskliutas commented 5 days ago

Describe the bug Because all packages in this repo are released in lockstep and they are using @internal APIs between each other, workspace peerDependencies should be specified without ^ to enforce that same version of packages will be installed by the package manager. At the moment all workspace peerDependencies are specified with ^ and package manager might resolve some of them to a different (higher) version if auto-install-peers are enabled.

Specifying peerDependencies without ^ would result in warning/error during installation.

To Reproduce Steps to reproduce the behavior:

  1. Go to https://stackblitz.com/edit/vitejs-vite-rthgsj?file=package.json&terminal=dev
  2. No warnings are reported during install regarding different @itwin/core-frontend and @itwin/core-common versions.

Expected behavior Warning or error should be produced by package manager during installation if different versions of locked step packages are installed.

grigasp commented 5 days ago

@pmconne @aruniverse, 5.0 seems like a good time to fix this oversight. We're willing to make the change, but would first like to get your agreement that this needs to be fixed.

aruniverse commented 5 days ago

fyi @wgoehrig

saskliutas commented 2 hours ago

I made one more sample to show why peerDependencies between itwinjs-core packages should be specified without ^. As we publish packages from itwinjs-core in lockstep we assume that consumers will use same version of those packages. Looking at package.json files this requirement is no clear and package managers do not try to resolve all packages to the same version.

For example @itwin/core-frontend@4.0.0 specifies peer dependency on @itwin/core-common as ^4.0.0 which means that it supports versions >4.0.0 and <5.0.0. However, trying to build app with @itwin/core-frontend@4.0.0 and @itwin/core-common@4.10.0 results in build failure although package manager was happy while installing dependencies. This can be seen be cloning peer-deps-fix branch from this repo: https://github.com/saskliutas/itwin-viewer-app/tree/peer-deps-fix and running pnpm install pnpm build.

This is quite an extreme example but it can be encountered accidentally if auto-install-peers flag is set to true and we add only @itwin/core-frontend@4.0.0 to package.json file. Package manager will resolve @itwin/core-common to the latest version based on ^4.0.0 specifier.

This could be mitigated by removing ^ from peerDependencies specifiers between itwinjs-core packages. In that case we would explicitly declare in package.json that @itwin/core-frontend@4.0.0 works and should be used only with @itwin/core-common@4.0.0. If different version are specified package manager will produce warning/error during installation. This can be seen in the example repo by setting fixDeps = true in .pnpmfile.cjs. It will remove ^ from peerDependencies between itwinjs-core package during install and running pnpm install will result in error.