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

Workspace peerDependencies should be specified without ^ #7373

Open
saskliutas opened this issue Nov 14, 2024 · 3 comments
Open

Workspace peerDependencies should be specified without ^ #7373

saskliutas opened this issue Nov 14, 2024 · 3 comments

Comments

@saskliutas
Copy link
Member

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
Copy link
Member

grigasp commented Nov 14, 2024

@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
Copy link
Member

fyi @wgoehrig

@saskliutas
Copy link
Member Author

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/[email protected] 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/[email protected] and @itwin/[email protected] 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/[email protected] 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/[email protected] works and should be used only with @itwin/[email protected]. 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.

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

No branches or pull requests

3 participants