[Desktop] [Web] Update templates to react 18 #287
Conversation
Jake-Screen
requested review from
aruniverse,
ben-polinsky,
calebmshafer and
a team
as code owners
|
Tested desktop and web viewer templates with these changes. The resizeObserver loop error no longer occurs with this update. |
| { | ||
| "packageName": "@itwin/cra-template-desktop-viewer", | ||
| "comment": "Update to React 18, see [their changelog](https://github.com/facebook/react/blob/main/CHANGELOG.md)", | ||
| "type": "patch" |
There was a problem hiding this comment.
My thought process was that we just bumped the minor when we merged in the react 18 upgrade for the rest of the repo, so this is just more along the same line
There was a problem hiding this comment.
Not quite following. Wouldn't "more along the same line" mean this should be minor as well?
There was a problem hiding this comment.
I mean that as in this should have been included in the previous pr. Should this warrant another minor version bump if it is only including the update from the previous pr?
There was a problem hiding this comment.
Ah that was one of the interpretations :) If we are releasing these together, then only one minor change will occur - they're all grouped together. At any rate, the type of change shouldn't be dependent on other changes. If it's a minor change, then it's a minor change. Even if we released and published your first PR changes, someone could download that version. Then we make this change which is minor by definition (if I am correct), but only marked as a patch. The next time that user goes to install, even if they have their minor version pinned, they'll get this update.
There was a problem hiding this comment.
Changed to minor
common/changes/@itwin/cra-template-web-viewer/jake-template-update_2023-07-19-15-21.json
Outdated
Show resolved
Hide resolved
|
@ben-polinsky is right that i would consider this a minor change in the template as well |
|
|
||
| root.render( | ||
| <React.StrictMode> | ||
| <App /> |
There was a problem hiding this comment.
does the desktop test app actually work in react 18 with strict mode?
i noticed some issues with the acr pkgs on my end
There was a problem hiding this comment.
It is working for me both as the test app in the repo and as the updated template. What specific errors are you seeing?
There was a problem hiding this comment.
im using the 1.0.0 version of the ACR pkgs, and they dont actually seem to fetch the itwins/imodels in the grids
There was a problem hiding this comment.
from the errors when upgrading to @itwin/imodel-browser-react it looks like the 1.0.0 version removes the support for ProjectGrid. Upgrading to the 1.0.0 version will have to wait for my other pr moving to the itwins api to finish.
Table this for now and complete the other pr first?
Updating desktop and web templates to react 18 and change to using
createRootto render applications