-
Notifications
You must be signed in to change notification settings - Fork 332
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
fix(react): fix compatibility issues with React 18 #969
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 54e6718:
|
Haroenv
reviewed
May 10, 2022
Haroenv
approved these changes
May 10, 2022
dhayab
approved these changes
May 10, 2022
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.
Cool workaround, thank you!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This introduces a documented solution to make
autocomplete-js
work with React 18.This solution is a workaround. We need to work on better support for these new React APIs, but this temporary solution allows React 18 users to work with Autocomplete in a non-breaking way.$
fixes #950
Documented solution
React 18 no longer recommends using
ReactDOM.render
, users should now be creating aRoot
object withReactDOM.createRoot
, then use therender
function on this object.In Autocomplete, we don't render in
panelContainer
but in an inner container (.aa-Panel
). Unlikecontainer
andpanelContainer
, this element is not injectable and is only accessible inrender
(it's the second parameterroot
). Therefore, we can't create the ReactRoot
object ahead of time and pass itsrender
function torenderer.render
.The solution passes
renderer.createElement
andrenderer.Fragment
, and implementsrender
instead of passingrenderer.render
. We create a ReactRoot
object withcreateRoot(root)
and store it in a ref to reuse it whenever we need to render.The
root
element can change (notably when switching between non-detached and detached mode) so we need to create a new ReactRoot
when it does. We do this by trackingroot
in a ref and comparing it wheneverrender
is called.Types
React 18's
createElement
was no longer assignable toPragma
because of two issues:ComponentChild
was assignable toobject
(this comes from Preact) whileReactNode
isn't.ReactNode
is assignable toReactPortal
which has a mandatorykey
thatVNode
didn't have.Regarding
object
, we don't need/use it internally so I think it's fine to drop it.Regarding
VNode
, I checked the latest version of the type in the Preact codebase and it looks likekey
is now mandatory at the root level (since preactjs/preact#1447). However I don't think there's ever been an optionalkey
inVNode.props
so I assumed this was added here by mistake instead of at the root (cc @francoischalifour) and I removed it.Type check is passing and our React InstantSearch Hooks example (using React 17) doesn't show type errors, so it looks fine.
Warning
Since this solution requires not passing
renderer.render
and implementingrender
instead, the following warning shows up:This warning is useful for React 17 and Vue users who implemented a custom renderer before 1.6.0 but it will become annoying for React 18 users.
I don't think we can detect the React version fromcreateElement
orFragment
, so I don't think we can selectively display the warning.React 18 will increasingly become the top users of therenderer
API so I believe it makes sense to cater to them. It comes at the expense of delivering this bit of advice, but it's not causing any issues so I think it's preferable drop it for now.As recommended by @Haroenv, we can recommend React 18 users to pass an empty function to
renderer.render
to dismiss the warning.