spike: react 18 update #925
Conversation
|
Thanks for the contribution! Unfortunately we can't verify the commit author(s): Robert Engel <r***@e***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, sign the Salesforce.com Contributor License Agreement and this Pull Request will be revalidated. |
| await waitFor(() => { | ||
| expect(result.current.mutation.isSuccess).not.toBe(initialSuccess) | ||
| }) | ||
|
|
||
| expect(result.current.mutation.isSuccess).toBe(true) | ||
|
|
||
| // On successful mutation, the query cache gets updated too. Let's assert it. |
There was a problem hiding this comment.
The assertion for invalidate?.forEach and remove?.forEach fail. To be honest I don't really know why. 🤷♂️
| @@ -198,7 +198,7 @@ RecommendedProducts.propTypes = { | |||
| recommender: PropTypes.string, | |||
|
|
|||
| /* The product IDs to use for recommendation context */ | |||
| products: PropTypes.arrayOf(PropTypes.string), | |||
| products: PropTypes.arrayOf(PropTypes.object), | |||
There was a problem hiding this comment.
This is not actually part of the needed changes but an existing error. I just fixed it to have a clean console to verify that pwa-kit is behaving as expected
There was a problem hiding this comment.
@ecRobertEngel Thanks so much for working on this! 🙏 Two things:
- Make sure you sign the CLA if we're going to merge this
- @natemarcus (our product manager) will need to set the date for this release so that we can synchronize other
release/v3changes we expect to ship along with this since the React 18 upgrade is a breaking change in the SDK
|
Currently the following issues with tests are still unresolved: pwa-kit search/index.test.js - renders Popover if recent searches populated: Only finds one element. use-auth-modal.test.js - Allows customer to create an account: callback is not getting invoked. cart/index.test.js - Can apply and remove product-level coupon code with promotion: finds element even though it should be removed. cart/index.test.jst - Can update item quantity from product view modal: Increment does not seem to be triggered cart/index.test.jst - Can update item quantity in the cart: Increment does not seem to be triggered account/index.test.js - Allows customer to sign out: Fetch error commerce-react-sdk mutation.test.ts - The checks for assertInvalidateQuery and assertRemoveQuery fail As I said as we use cypress component tests in our project I don't have too much experience with the jest tests here. So maybe the issues are obvious to a more experienced dev here. |
|
There are two things that are still causing hydration issues. They are related to the use of Steps to reproduce either : Navigate to a PLP from client side and reload or navigate directly to a PLP page If the isServer logic is removed both work without issue. There might be changes needed to handle this. For the moment I won't touch it here. |
…dow object checks
The solution is to use a hook instead of the checks against the window global in the component. A possible hook has been added to the PR. |
|
@ecRobertEngel Wanted to say thank you again for working on this and the work will definitely be useful to us soon. I've added the @natemarcus we have our own internal backlog but for those parts of v3 that are requested by customers (like this) I've added to https://github.com/SalesforceCommerceCloud/pwa-kit/milestone/1 |
|
Closing this for now. Looks like it'll be useful later for our v3 work. |
Proof of concept implementation of a React 18 and ChakraUI 2 upgrade
Description
Please be aware that this is not intended to be merged.
As I had to update pwa-kit for a project to React 18 I wanted to share my findings with you. While the template-retail-react-app is running very stable there are still issues with test files that I have not resolved.
Most important change for the upgrade is the change in the
main.jsxwith the changed hydration API that requires a manual callbackSome notes about testing:
Types of Changes
Changes
How to Test-Drive This PR
Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization