-
Notifications
You must be signed in to change notification settings - Fork 143
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
Upgrade to React 18 #1166
Conversation
packages/template-retail-react-app/app/pages/product-list/partials/page-header.jsx
Show resolved
Hide resolved
packages/template-retail-react-app/app/pages/product-detail/partials/information-accordion.jsx
Show resolved
Hide resolved
packages/template-retail-react-app/app/components/forms/useCreditCardFields.jsx
Show resolved
Hide resolved
packages/template-retail-react-app/app/hooks/use-is-hydrated.js
Outdated
Show resolved
Hide resolved
packages/template-retail-react-app/app/theme/components/project/breadcrumb.js
Outdated
Show resolved
Hide resolved
] | ||
], | ||
"overrides": { | ||
"nwsapi": "2.2.2" |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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) => { |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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)
})
There was a problem hiding this 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') |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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', () => { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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', () => { | |||
*/ |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this 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
There was a problem hiding this 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! 🚀
* 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
* 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
…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
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:
The following packages are removed as the peerDependencies are not <= React 17
Types of Changes
Changes
':disabled):not([disabled]' is not a valid selector
on some of the tests that hasModal
. 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 supportsetup
before using itHow to Test-Drive This PR
npm ci
at rootChecklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization