Skip to content

[Fix] : Template issue#361

Merged
aruniverse merged 2 commits intomasterfrom
pankhur94/template-bug-fix
May 20, 2025
Merged

[Fix] : Template issue#361
aruniverse merged 2 commits intomasterfrom
pankhur94/template-bug-fix

Conversation

@pankhur94
Copy link
Copy Markdown
Contributor

@pankhur94 pankhur94 commented May 16, 2025

Removes @itwin/presentation-core-interop as direct dep and override for metadependencies to 1.3.1.

@pankhur94
Copy link
Copy Markdown
Contributor Author

pankhur94 commented May 16, 2025

I could not find any instance of direct usage of the package @itwin/presentation-core-interop directly in the app and hence took the liberty to remove it. If we need it as a direct dep, we would have to ask the user to manually change it to 1.3.1 as bentley react scripts prefixes "^" which leads to conflict with the override here. This is standard behaviour of npm until we explicitly specify --save-exact while running npm install or add save-prefix= in .npmrc file. The user would then have to manually add below in package.json as well before npm install.

overrides : {
"@itwin/presentation-core-interop": "1.3.1"
}

@aruniverse
Copy link
Copy Markdown
Member

@itwin/presentation-core-interop must be a peer dep of some other pkg, which is why we need to include it in our end
Review the blame here: https://github.com/iTwin/viewer/blame/master/packages/templates/cra-template-web-viewer/template.json#L30

@pankhur94
Copy link
Copy Markdown
Contributor Author

pankhur94 commented May 16, 2025

@itwin/presentation-core-interop must be a peer dep of some other pkg, which is why we need to include it in our end Review the blame here: https://github.com/iTwin/viewer/blame/master/packages/templates/cra-template-web-viewer/template.json#L30

I did check and found out that it was added in this commit. However I could not find any of the other dependencies added/updated to have it as a peer-dependency. To confirm I ran npm ls @itwin/presentation-core-interop --all in the newly created app and did not find @itwin/presentation-core-interop to be a peer dependency in any of the deps either.
image
Also, it is not listed as a peer dependency in any of our deps in package-lock.json.

@grigasp
Copy link
Copy Markdown
Member

grigasp commented May 19, 2025

I agree, it looks like it's unnecessary, I think adding it was just an oversight.

However, I don't understand why the following part is necessary:

    "overrides": {
      "@itwin/presentation-core-interop": "1.3.1"
    }

@pankhur94
Copy link
Copy Markdown
Contributor Author

pankhur94 commented May 20, 2025

I agree, it looks like it's unnecessary, I think adding it was just an oversight.

However, I don't understand why the following part is necessary:

    "overrides": {
      "@itwin/presentation-core-interop": "1.3.1"
    }

introduction of checking SchemaFormatsProviders here in 1.3.2 is causing errors in viewer since webpack tries to statically resolve all imports. Hence, we need to make sure we use 1.3.1 to fix this error.
We can remove this once we have moved completely to use 5.0

@pankhur94 pankhur94 marked this pull request as ready for review May 20, 2025 14:01
@aruniverse
Copy link
Copy Markdown
Member

But how are these overrides applied? does npm automatically apply them?

@pankhur94
Copy link
Copy Markdown
Contributor Author

But how are these overrides applied? does npm automatically apply them?

yes, that is right, npm understands overrides and when it does npm install, it applies them.

@aruniverse aruniverse added this pull request to the merge queue May 20, 2025
Merged via the queue into master with commit eb0f9bb May 20, 2025
4 checks passed
@aruniverse aruniverse deleted the pankhur94/template-bug-fix branch May 20, 2025 14:24
aruniverse added a commit that referenced this pull request Jun 11, 2025
* point to new reality data endpoint (#345)

* point to new reality data endpoint

* rush change

---------

Co-authored-by: Arun George <aruniverse@users.noreply.github.com>

* Update changelogs [skip ci]

* Bump versions [skip ci]

* mitigate audit GHSA-rhx6-c78j-4q9w (#347)

Co-authored-by: Ben Polinsky <ben-polinsky@users.noreply.github.com>

* remove custom audit script, resolve GHSA-jr5f-v2jv-69x6 (#348)

* remove custom audit script, resolve GHSA-jr5f-v2jv-69x6

* suppress deprecations for now, will fix on 5.0

---------

Co-authored-by: Arun George <aruniverse@users.noreply.github.com>

* Stop using deprecated selection APIs (#346)

* update dependencies

* change usage of  to

* update property widget to the latest, supply it the unified selection storage

* allow specifying selection scope props

* update selected items count without using deprecated apis

* downgrade `@itwin/unified-selection` dependency to `~1.1.2`

* change

* prettier

* fix tests

* Extract nested selection scope props type into , define unified selection props using  type helper

* fix change messages

* update lockfile

---------

Co-authored-by: Ben Polinsky <78756012+ben-polinsky@users.noreply.github.com>
Co-authored-by: Arun George <aruniverse@users.noreply.github.com>

* Update changelogs [skip ci]

* Bump versions [skip ci]

* Fix desktop template build issue (#349)

* Fix desktop app build issue

* rush change

* Update changelogs [skip ci]

* Bump versions [skip ci]

* Add missing peer dependencies (#356)

* Add missing peer dependencies

* rush change

* Update changelogs [skip ci]

* Bump versions [skip ci]

* [Fix]: iTwin Viewer template issue (#359)

* adding unified-selection-react as dep

* rush change

* adding comment and type to rush change

* Update changelogs [skip ci]

* Bump versions [skip ci]

* [Fix] : Template issue (#361)

* removing presentation-core-interop as direct dep, adding override to version 1.3.1

* rush change

* Update changelogs [skip ci]

* Bump versions [skip ci]

* [ Fix ]: Desktop viewer template fix (#364)

* removing @itwin/presentation-core-interop and overriding version 1.3.1

* rush change

* fixing comment in json

* Update changelogs [skip ci]

* Bump versions [skip ci]

* web and desktop template fix

* update unified-selection, browser-authorization, fix lint

* updating deps

* update and clean deps, fix context

* comments resolved

---------

Co-authored-by: Arun George <11051042+aruniverse@users.noreply.github.com>
Co-authored-by: Arun George <aruniverse@users.noreply.github.com>
Co-authored-by: imodeljs-admin <imodeljs-admin@users.noreply.github.com>
Co-authored-by: Ben Polinsky <78756012+ben-polinsky@users.noreply.github.com>
Co-authored-by: Ben Polinsky <ben-polinsky@users.noreply.github.com>
Co-authored-by: Grigas <35135765+grigasp@users.noreply.github.com>
Co-authored-by: Gytis Čepkauskas <98940208+GytisCepk@users.noreply.github.com>
Co-authored-by: Saulius Skliutas <24278440+saskliutas@users.noreply.github.com>
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 this pull request may close these issues.

4 participants