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

Ponyfill useId #88

Merged
merged 3 commits into from
Sep 26, 2023
Merged

Ponyfill useId #88

merged 3 commits into from
Sep 26, 2023

Conversation

@Johennes Johennes requested a review from a team as a code owner September 26, 2023 05:52
@Johennes Johennes requested review from dbkr and kerryarchibald and removed request for a team September 26, 2023 05:52
@Johennes Johennes mentioned this pull request Sep 26, 2023
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Sep 26, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7898f19
Status: ✅  Deploy successful!
Preview URL: https://f1defd41.compound-web.pages.dev
Branch Preview URL: https://johannes-pony-fill-use-id.compound-web.pages.dev

View logs

@Johennes Johennes requested review from germain-gg and removed request for dbkr and kerryarchibald September 26, 2023 05:54
@Johennes
Copy link
Contributor Author

Percy is now failing because of the quota.

[percy] Error: This organization has exceeded the limits of the Percy BrowserStack plan. Administrators can upgrade here: https://percy.io/organizations/c8fecada/billing

src/utils/useId.ts Show resolved Hide resolved
Copy link
Contributor

@germain-gg germain-gg left a comment

Choose a reason for hiding this comment

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

Code looks good to me,

Could we add an ESLint rule to make useId a restricted import and suggest using your ponyfill instead?
I believe we're using https://eslint.org/docs/latest/rules/no-restricted-imports on EW

Does not have to block this pull request, but a quick follow up would be appreciated

@Johennes
Copy link
Contributor Author

Ok, let me see if I can get that going tonight. Will land this in the interim.

@Johennes
Copy link
Contributor Author

Sorry, I didn't get around to do this last night. I have put in element-hq/compound#228 and will try to come back to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useId not available in React 17
3 participants