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

Upgrade to React 18 #1166

Merged
merged 55 commits into from May 15, 2023
Merged

Upgrade to React 18 #1166

merged 55 commits into from May 15, 2023

Conversation

alexvuong
Copy link
Collaborator

@alexvuong alexvuong commented May 2, 2023

Description

This PR upgraded React to version 18, and Chakra version 2 and any related library to be compatible with React 18. The list of upgraded package includes:

  • React 18.2.0
  • React-dom 18.2.0
  • "@types/react": "^18.2.0",
  • "@types/react-dom": "^18.2.1",
  • "@testing-library/react": "^14.0.0",
  • "@testing-library/user-event": "^14.4.3",
  • "@chakra-ui/icons": "^2.0.19",
  • "@chakra-ui/react": "^2.6.0",
  • "@chakra-ui/skip-nav": "^2.0.15",
  • "@chakra-ui/system": "^2.5.6",
  • "react-hook-form": "^7.43.9",
  • "framer-motion": "^4.1.17",

The following packages are removed as the peerDependencies are not <= React 17

  • @testing-library/render-hooks
  • @wojtekmaj/enzyme-adapter-react-17

Types of Changes

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Documentation update
  • Breaking change (could cause existing functionality to not work as expected)
  • Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

  • Removing a public function or component or prop
  • Adding a required argument to a function
  • Changing the data type of a function parameter or return value
  • Adding a new peer dependency to package.json

Changes

  • HydrateRoot API has changed. We cant pass callback function as we are doing. Instead, we can pass a prop into the Component to have it called after it is created. See here
  • Fix hydration error by creating a utils function to use window.HYDRATING. See here
  • React hook form v7
    • errors object has been moved into formState object. see here
    • render prop will return an object which contains field and fieldState. See here
  • For unit tests changes:
    • There is a bug in a dependency of a dependency, nswapi, which is a dependency of jest-environment-jsdom that is not compatible with Chakra 2. It will throw an error ':disabled):not([disabled]' is not a valid selector on some of the tests that has Modal. Solution: we use npm 8, and add overrides to enforce the version of nwsapi package to 2.2.2, and we have to drop npm 7 support
    • UserEvent actions are now async action, it will always return promise , and we have to call setup before using it
import userEvent from '@testing-library/user-event
test('mytest' async () => {
     const user = userEvent.setup()
     render(<Component />)
     
     
    await  user.click(screen.getByText(/something/i)
})

How to Test-Drive This PR

  • npm ci at root
  • Start up Retail App
  • Smoke test the app, and make sure it behaves as usual

Checklists

General

  • Changes are covered by test cases
  • CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

  • There are no changes to UI

or...

Localization

  • Changes include a UI text update in the Retail React App (which requires translation)

@alexvuong alexvuong marked this pull request as ready for review May 5, 2023 23:53
@alexvuong alexvuong requested a review from a team as a code owner May 5, 2023 23:53
@alexvuong alexvuong changed the title upgrade to React 18 Upgrade to React 18 May 5, 2023
@alexvuong alexvuong self-assigned this May 6, 2023
@alexvuong alexvuong added the ready for review PR is ready to be reviewed label May 6, 2023
]
],
"overrides": {
"nwsapi": "2.2.2"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to enforce this package in Retail App for the tests in generated project to pass. This should only affect unit tests, and not impact product code

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wish json support comments

* }
* ]
*/
export const prependHandlersToServer = (handlerConfig) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a util function to allow us to append mock request handler into global.server mock to reduce code verbosity, but I haven't applied it to all the mock we have in tests, only a few that I fixed in this PR. We can refactor those as we see fit.

@@ -182,16 +182,18 @@ export const renderWithProviders = (children, options) => {
}
}
})
const user = userEvent.setup()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in React testing library v14.0.0, all the user actions are async, and it requires calling setup before performing user actions. To avoid repetitive setup, we can set it up here, right before rendering the test component, and return it along with render results, so the test can perform the user actions without having to call setup.

test('some test", async () => {
  const {user} = renderWithProvider(<Comp />

  await  user.click(screen.getByText*/some text/i) 
})

package.json Show resolved Hide resolved
kevinxh
kevinxh previously approved these changes May 15, 2023
Copy link
Collaborator

@adamraya adamraya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comments on the enzyme work

expect(scriptTag).toBeInTheDocument()
expect(styleTag).toBeInTheDocument()
expect(screen.getByText(/hello world/i)).toBeInTheDocument()
expect(bodyTag).toHaveAttribute('class', 'root')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to be consistent with the previous testing, note that now we're missing testing the lang attribute in the HTML tag.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that, it has been addressed, see 2f69f6a#diff-cd330e53513b04d5b7e6fabbf75f8df0aed7f3b684c5019f7a416ff1bc0f65e6R46

expect(wrapper.props().status).toBe(status)
})

test('Ensure that status type is a number', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're missing this test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We intentionally removed this test since React Testing library do not focus on the prop type, rather than focus on what components are rendering, and we want to see on the DOM.

@@ -292,12 +307,12 @@ describe('getRoutes', () => {
*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description mentions the usage of setProps enzyme API, but we no longer use that API.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah thanks for catching that, in enzyme, setProps is used to trigger prop change on a test component, in React testing library, we can do the same trick by using rerender method returned from the first time we render the component

bfeister
bfeister previously approved these changes May 15, 2023
@bfeister bfeister self-requested a review May 15, 2023 21:38
@alexvuong alexvuong dismissed stale reviews from bfeister and kevinxh via 63d5656 May 15, 2023 21:38
Copy link
Collaborator

@bfeister bfeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss / resolve @adamraya 's comments and then we can merge

bfeister
bfeister previously approved these changes May 15, 2023
Copy link
Collaborator

@bfeister bfeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for resolving all the comments. Great work! 🏅 LGTM! 🚀

kevinxh
kevinxh previously approved these changes May 15, 2023
@alexvuong alexvuong dismissed stale reviews from kevinxh and bfeister via f149f3c May 15, 2023 22:01
@alexvuong alexvuong enabled auto-merge (squash) May 15, 2023 22:21
@alexvuong alexvuong merged commit 7bc39d3 into develop May 15, 2023
20 checks passed
bfeister added a commit that referenced this pull request May 23, 2023
* develop: (37 commits)
  [Phased Launch] Call Session bridge after login (#1220)
  [v3] Add multi-site suffix to auth token keys (#1208)
  Footer: fix hydration error with the locale dropdown (#1210)
  remove unused peerDependency @chakra-ui/system (#1212)
  @W-12582733: Expose env vars endpoint for E2E smoke tests (#1207)
  Upgrade to React 18 (#1166)
  Remove v3 branch name related actions (#1206)
  add test to reach test coverage threshold
  remove commerce-api folder
  Feature: Extract einstein RefArch-specific values to constant (#1200)
  Bump version number to 2.7.1 and update changelogs (#1197)
  store usid in cookies (#1193)
  make sure static files are copied on dev environment (#1196)
  [WIP] PWA Kit 2.7.1 release (#1181)
  [V2] Internal lib build typescript dev dependency (#1194)
  [V2] Re-generate lock files and fix hook lib tests (#1186)
  [v3] Add Convenience Components (#1183)
  Update commerce-sdk-react README (#1176)
  Add additional properties to ShopperLogin test types (#1185)
  Fix missing commerce-sdk-react logout call (#1180)
  ...

# Conflicts:
#	package-lock.json
#	packages/commerce-sdk-react/CHANGELOG.md
#	packages/commerce-sdk-react/package-lock.json
#	packages/internal-lib-build/package-lock.json
#	packages/pwa-kit-create-app/package-lock.json
#	packages/pwa-kit-dev/package-lock.json
#	packages/pwa-kit-react-sdk/package-lock.json
#	packages/pwa-kit-runtime/package-lock.json
#	packages/template-mrt-reference-app/package-lock.json
#	packages/template-retail-react-app/package-lock.json
#	packages/template-typescript-minimal/package-lock.json
#	packages/test-commerce-sdk-react/package-lock.json
bfeister added a commit that referenced this pull request May 23, 2023
* develop: (34 commits)
  [Phased Launch] Call Session bridge after login (#1220)
  [v3] Add multi-site suffix to auth token keys (#1208)
  Footer: fix hydration error with the locale dropdown (#1210)
  remove unused peerDependency @chakra-ui/system (#1212)
  @W-12582733: Expose env vars endpoint for E2E smoke tests (#1207)
  Upgrade to React 18 (#1166)
  Remove v3 branch name related actions (#1206)
  add test to reach test coverage threshold
  remove commerce-api folder
  Feature: Extract einstein RefArch-specific values to constant (#1200)
  Bump version number to 2.7.1 and update changelogs (#1197)
  store usid in cookies (#1193)
  make sure static files are copied on dev environment (#1196)
  [WIP] PWA Kit 2.7.1 release (#1181)
  [V2] Internal lib build typescript dev dependency (#1194)
  [V2] Re-generate lock files and fix hook lib tests (#1186)
  Add additional properties to ShopperLogin test types (#1185)
  Revert 2.7.0 branch to prep for 2.7.1 changes (#1182)
  #1174 Replace invalid value for wrap property (#1179)
  Add a redirect to login page after user signs out from checkout page (#1172)
  ...

# Conflicts:
#	package-lock.json
#	packages/commerce-sdk-react/CHANGELOG.md
#	packages/commerce-sdk-react/package-lock.json
#	packages/internal-lib-build/package-lock.json
#	packages/pwa-kit-create-app/package-lock.json
#	packages/pwa-kit-dev/package-lock.json
#	packages/pwa-kit-dev/package.json
#	packages/pwa-kit-react-sdk/package-lock.json
#	packages/pwa-kit-runtime/package-lock.json
#	packages/template-mrt-reference-app/package-lock.json
#	packages/template-retail-react-app/app/components/confirmation-modal/index.test.js
#	packages/template-retail-react-app/app/components/footer/index.jsx
#	packages/template-retail-react-app/app/components/header/index.jsx
#	packages/template-retail-react-app/app/components/list-menu/index.test.js
#	packages/template-retail-react-app/app/components/product-scroller/index.test.js
#	packages/template-retail-react-app/app/components/product-view-modal/index.test.js
#	packages/template-retail-react-app/app/components/search/index.test.js
#	packages/template-retail-react-app/app/hoc/with-registration/index.test.js
#	packages/template-retail-react-app/app/hooks/use-add-to-cart-modal.js
#	packages/template-retail-react-app/app/hooks/use-auth-modal.test.js
#	packages/template-retail-react-app/app/hooks/use-currency.test.js
#	packages/template-retail-react-app/app/hooks/use-multi-site.test.js
#	packages/template-retail-react-app/app/hooks/use-navigation.test.js
#	packages/template-retail-react-app/app/pages/account/addresses.test.js
#	packages/template-retail-react-app/app/pages/account/index.jsx
#	packages/template-retail-react-app/app/pages/account/wishlist/partials/wishlist-primary-action.test.js
#	packages/template-retail-react-app/app/pages/cart/index.test.js
#	packages/template-retail-react-app/app/pages/cart/partials/cart-secondary-button-group.test.js
#	packages/template-retail-react-app/app/pages/checkout/index.test.js
#	packages/template-retail-react-app/app/pages/checkout/partials/contact-info.jsx
#	packages/template-retail-react-app/app/pages/checkout/partials/contact-info.test.js
#	packages/template-retail-react-app/app/pages/home/index.test.js
#	packages/template-retail-react-app/app/pages/product-list/partials/empty-results.jsx
#	packages/template-retail-react-app/app/pages/product-list/partials/page-header.jsx
#	packages/template-retail-react-app/app/pages/registration/index.test.jsx
#	packages/template-retail-react-app/app/utils/site-utils.js
#	packages/template-retail-react-app/package-lock.json
#	packages/template-typescript-minimal/package-lock.json
#	packages/test-commerce-sdk-react/package-lock.json
bfeister added a commit that referenced this pull request May 24, 2023
…react-app

* feature/template-extensibility: (38 commits)
  support windows file paths
  Feature/template extensibility (#1162)
  lockfiles from reaact18 / chakra2
  remove demo extensible app in light of soon-to-be-merged PR from @bendvc
  [Phased Launch] Call Session bridge after login (#1220)
  [v3] Add multi-site suffix to auth token keys (#1208)
  Footer: fix hydration error with the locale dropdown (#1210)
  remove unused peerDependency @chakra-ui/system (#1212)
  @W-12582733: Expose env vars endpoint for E2E smoke tests (#1207)
  Upgrade to React 18 (#1166)
  Remove v3 branch name related actions (#1206)
  add test to reach test coverage threshold
  remove commerce-api folder
  Feature: Extract einstein RefArch-specific values to constant (#1200)
  Bump version number to 2.7.1 and update changelogs (#1197)
  store usid in cookies (#1193)
  make sure static files are copied on dev environment (#1196)
  [WIP] PWA Kit 2.7.1 release (#1181)
  [V2] Internal lib build typescript dev dependency (#1194)
  [V2] Re-generate lock files and fix hook lib tests (#1186)
  ...

# Conflicts:
#	packages/my-extended-retail-app/config/default.js
#	packages/my-extended-retail-app/config/sites.js
#	packages/my-extended-retail-app/package-lock.json
#	packages/my-extended-retail-app/package.json
#	packages/my-extended-retail-app/pwa-kit-overrides/app/assets/svg/brand-logo.svg
#	packages/my-extended-retail-app/pwa-kit-overrides/app/components/_app-config/index.jsx
#	packages/my-extended-retail-app/pwa-kit-overrides/app/components/_app/index.jsx
#	packages/my-extended-retail-app/pwa-kit-overrides/app/pages/home/index.jsx
#	packages/my-extended-retail-app/pwa-kit-overrides/app/routes.jsx
#	packages/my-extended-retail-app/pwa-kit-overrides/app/ssr.js
#	packages/my-extended-retail-app/pwa-kit-overrides/app/static/ico/favicon.ico
#	packages/my-extended-retail-app/pwa-kit-overrides/app/static/img/global/app-icon-192.png
#	packages/my-extended-retail-app/pwa-kit-overrides/app/static/img/global/app-icon-512.png
#	packages/my-extended-retail-app/pwa-kit-overrides/app/static/img/global/apple-touch-icon.png
#	packages/pwa-kit-dev/bin/pwa-kit-dev.js
#	packages/pwa-kit-dev/package-lock.json
#	packages/pwa-kit-dev/package.json
#	packages/pwa-kit-dev/src/configs/webpack/config.js
#	packages/template-retail-react-app/app/components/confirmation-modal/index.test.js
#	packages/template-retail-react-app/app/components/footer/index.jsx
#	packages/template-retail-react-app/app/components/header/index.jsx
#	packages/template-retail-react-app/app/components/list-menu/index.test.js
#	packages/template-retail-react-app/app/components/product-scroller/index.test.js
#	packages/template-retail-react-app/app/components/product-view-modal/index.test.js
#	packages/template-retail-react-app/app/components/search/index.test.js
#	packages/template-retail-react-app/app/hoc/with-registration/index.test.js
#	packages/template-retail-react-app/app/hooks/use-add-to-cart-modal.js
#	packages/template-retail-react-app/app/hooks/use-auth-modal.test.js
#	packages/template-retail-react-app/app/hooks/use-currency.test.js
#	packages/template-retail-react-app/app/hooks/use-multi-site.test.js
#	packages/template-retail-react-app/app/hooks/use-navigation.test.js
#	packages/template-retail-react-app/app/pages/account/addresses.test.js
#	packages/template-retail-react-app/app/pages/account/index.jsx
#	packages/template-retail-react-app/app/pages/account/wishlist/index.test.js
#	packages/template-retail-react-app/app/pages/account/wishlist/partials/wishlist-primary-action.test.js
#	packages/template-retail-react-app/app/pages/cart/index.test.js
#	packages/template-retail-react-app/app/pages/cart/partials/cart-secondary-button-group.test.js
#	packages/template-retail-react-app/app/pages/checkout/index.test.js
#	packages/template-retail-react-app/app/pages/checkout/partials/contact-info.jsx
#	packages/template-retail-react-app/app/pages/checkout/partials/contact-info.test.js
#	packages/template-retail-react-app/app/pages/home/index.test.js
#	packages/template-retail-react-app/app/pages/product-list/partials/empty-results.jsx
#	packages/template-retail-react-app/app/pages/product-list/partials/page-header.jsx
#	packages/template-retail-react-app/app/pages/registration/index.test.jsx
#	packages/template-retail-react-app/app/utils/site-utils.js
#	packages/template-retail-react-app/package-lock.json
#	packages/template-retail-react-app/package.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review PR is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants