-
Notifications
You must be signed in to change notification settings - Fork 560
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 auto-id
build for backwards compatibility with React <18
#961
base: dev
Are you sure you want to change the base?
Conversation
# storybook | ||
/playground/storybook-static |
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.
Artifact of building Storybook locally.
// TODO: Remove error flag when updating internal deps to React 18. None of | ||
// our tricks will play well with concurrent rendering anyway. | ||
// @ts-expect-error | ||
if (typeof React.useId === "function") { | ||
// @ts-expect-error | ||
let id = React.useId(providedId); | ||
return providedId != null ? providedId : id; | ||
} |
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.
This is what is causing the problem. I suggest to simply remove this once only React >=18 is supported.
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.
Is there no way to rewrite this to keep your build process from showing a warning? I think that would be preferable.
@@ -1,5 +1,5 @@ | |||
import * as React from "react"; | |||
import WindowSize from "@reach/window-size"; | |||
import { WindowSize } from "@reach/window-size"; |
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 import was wrong
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 a8aed9e:
|
Summary
Fixes #921
TL;DR: the bundle of
@reach/auto-id
contains an import ofuseId
fromreact
. For versions of React <18 this import does not exist, causing an error when this package is used.This can also be seen in the Storybook playground build output (although Storybook shows a warning)
To fix that, I suggest to remove that code and change the implementation once React >=18 is supported.
I also fixed another import issue shown by Storybook
Thank you for contributing to Reach UI! Please fill in this template before submitting your PR to help us process your request more quickly.
yarn test
).yarn start
).This pull request:
If creating a new package:
examples
directorysrc
directory with anindex.tsx
entry filestyle.css
file (if needed by the new package)