search

iTwin / appui

Monorepo for iTwin.js AppUi
MIT License
9 stars 2 forks source link

Move `@itwin/itwinui-react` to peerDependencies #1115

Closed GerardasB closed 2 weeks ago

GerardasB commented 2 weeks ago

Changes

This PR moves @itwin/itwinui-react to peerDependencies to simplify the process of ensuring that a single version of iTwinUI package is installed per major version. Will be backported to release/5.0.x branch where NextVersion.md will be updated to mention this change.

Testing

N/A

GerardasB commented 2 weeks ago

@Mergifyio backport release/5.0.x

mergify[bot] commented 2 weeks ago

backport release/5.0.x

✅ Backports have been created

* [#1118 Move `@itwin/itwinui-react` to peerDependencies (backport #1115)](https://github.com/iTwin/appui/pull/1118) has been created for branch `release/5.0.x` but encountered conflicts
aruniverse commented 1 week ago

Why does iTwinUI need to be a peer at all? Especially if we dropped support for v2, and now only supporting v3

grigasp commented 1 week ago

Why does iTwinUI need to be a peer at all? Especially if we dropped support for v2, and now only supporting v3

Because iTwinUI requires one major version of the package to be used across all components in the whole app. The only correct way to achieve that is if all components specify it as a peer dependency, and the actual version is specified by the top-level application's package.json.

Currently a lot of components incorrectly have it specified under dependencies with varying versions, which is the reason consumer applications need to use hacks to override whatever version the component requests to whatever version the application actually wants.

aruniverse commented 1 week ago

Currently a lot of components incorrectly have it specified under dependencies with varying versions

As long as they're using ^ wouldn't the package manager resolve all the versions to a single version at the app level?

saskliutas commented 1 week ago

Currently a lot of components incorrectly have it specified under dependencies with varying versions

As long as they're using ^ wouldn't the package manager resolve all the versions to a single version at the app level?

Not necessarily, from my experience If you add a new package into your app and there is newer version of itwinui-react available only that newly added package will use it. Other packages will be left with the version that was locked in lock-file before.

Other problem I see with having itiwnui-react as direct dependency is that it is harder for app to stick with specific itwinui-react version. For example: app uses itwinui-react@3.8.0 and there is a new compA package version with itwinui-react@^3.9.0. If app pulls in that new version it will end up with two versions of itwinui-react: 3.8.0 and ^3.9.0. This will not produce any warnings during install or build and would be visible only at the runtime. To resolve it app will probably end up with pnpm.overrides { itwinui-react: 3.8.0 } but compA might not work with itwinui-react@3.8.0 as it specifies itwinui-react version as ^3.9.0. So now app developer need to go into compA version history and find version that has itwinui-react <=3.8.0 and pin it.

On the other hand if compA had itwinui-react as peerDependency app would get at least a warning during install that itwinui-react@3.8.0 does not meet compA peerDep requirement of itwinui-react@^3.9.0.