Skip to content
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

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Expand Up @@ -12,6 +12,9 @@ node_modules
.env
coverage

# storybook
/playground/storybook-static
Comment on lines +15 to +16
Copy link
Author

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.


# website
/website-deploy-key
/website-deploy-key.pub
Expand Down
9 changes: 0 additions & 9 deletions packages/auto-id/src/auto-id.ts
Expand Up @@ -87,15 +87,6 @@ function useId(
function useId(): string | undefined;

function useId(providedId?: number | string | undefined | null) {
// 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;
}
Comment on lines -90 to -97
Copy link
Author

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.

Copy link

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.


// If this instance isn't part of the initial render, we don't have to do the
// double render/patch-up dance. We can just generate the ID and return it.
let initialId = providedId ?? (serverHandoffComplete ? genId() : null);
Expand Down
2 changes: 1 addition & 1 deletion playground/stories/window-size/basic.example.js
@@ -1,5 +1,5 @@
import * as React from "react";
import WindowSize from "@reach/window-size";
import { WindowSize } from "@reach/window-size";
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import was wrong


let name = "Basic";

Expand Down
2 changes: 1 addition & 1 deletion playground/stories/window-size/basic.example.tsx
@@ -1,5 +1,5 @@
import * as React from "react";
import WindowSize from "@reach/window-size";
import { WindowSize } from "@reach/window-size";

let name = "Basic (TS)";

Expand Down
2 changes: 1 addition & 1 deletion website/src/pages/window-size.mdx
Expand Up @@ -28,7 +28,7 @@ yarn add @reach/window-size
```

```js
import WindowSize, { useWindowSize } from "@reach/window-size";
import { WindowSize, useWindowSize } from "@reach/window-size";
```

## Component API
Expand Down