cauldron icon indicating copy to clipboard operation
cauldron copied to clipboard

Upgrade to React v18

Open stephenmathieson opened this issue 3 years ago • 6 comments

Once #1159 and #1473 is complete, we should upgrade to the latest version of React 18. This should include both react and react-dom for development dependencies in the following packages:

  • ./ (root)
  • packages/react

Peer dependencies can be changed to the following:

"peerDependencies": {
  "react": ">=16.6",
  "react-dom": ">=16.6"
}

Also see https://react.dev/blog/2022/03/08/react-18-upgrade-guide for a reference of changes that may be needed. Running the newly updated tests should help catch any lingering issues, but a full breadth testing of Cauldron documentation should be done to identify any possible issues with each component.

stephenmathieson avatar Apr 21 '22 15:04 stephenmathieson

There's a possibility that we could unofficially support React 17/18, I'm not seeing any breaking changes for 17. We could temporarily use this third party adapter to allow cauldron to use React 17. There may not be anything preventing someone from using cauldron in 18, we would just need to validate within the apps.

scurker avatar Jun 07 '22 15:06 scurker

It looks like there's some breaking changes with v18:

scurker avatar Jun 07 '22 16:06 scurker

The updates to the types are what's preventing us from upgrading axe.deque.com to React 18. We should get those sorted out ASAP.

stephenmathieson avatar Jun 07 '22 17:06 stephenmathieson

The modals break when you upgrade to react-dom 17. Modal toggle is called twice on open in 4.7.0-canary.a816b304 (fix from Pr #698) and four times with 4.7.0-canary.87de5e62. It appears that the onClose event is always called by clickOutsideListener probably because of changes to event delegation especially with document.addEventListener.

OptionsMenu also will not open.

abbeyperini-deque avatar Oct 03 '22 20:10 abbeyperini-deque

The modals break when you upgrade to react-dom 17. Modal toggle is called twice on open in 4.7.0-canary.a816b304 (fix from Pr #698) and four times with 4.7.0-canary.87de5e62. It appears that the onClose event is always called by clickOutsideListener probably because of changes to event delegation especially with document.addEventListener.

@schne324 and I noticed this yesterday on React@18, but I don't think it's necessarily directly related to the event delegation changes.

ClickOutsideListener does not use onClick handlers but manages its own events:

https://github.com/dequelabs/cauldron/blob/a816b304ac4667e3c3734d4147a044fcf975961f/packages/react/src/components/ClickOutsideListener/index.tsx#L70-L76

The issue I noticed is that addEventListener for ClickOutsideListener was somehow getting mounted before the triggering element causing the onClickOutside to be invoked with the triggering element. My guess between 16/18 something has changed with scheduling. It's possible that automatic batching could be the cause for react@18, but we need to investigate this further.

scurker avatar Oct 04 '22 15:10 scurker

🤔 e.stopPropagation() in the onClick in the modal before the modal toggle call prevented onClose from being called and now the modals work correctly.

Decided to swap to TopBarMenu and OptionsMenuList instead of debugging OptionsMenu.

abbeyperini-deque avatar Oct 04 '22 15:10 abbeyperini-deque