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

feat: support React 18 #164

Merged
merged 5 commits into from
Apr 11, 2022
Merged

feat: support React 18 #164

merged 5 commits into from
Apr 11, 2022

Conversation

andipaetzold
Copy link
Contributor

@andipaetzold andipaetzold commented Apr 5, 2022

React 18 doesn't have any relevant breaking changes.

closes #163
closes #166

@andipaetzold andipaetzold mentioned this pull request Apr 5, 2022
@bacali95
Copy link

bacali95 commented Apr 7, 2022

Hello @andipaetzold, thanks for adding support for React 18, just wanted to mention that the types of React do not use React.PropsWithChildren type by default for class and functional components see DefinitelyTyped/DefinitelyTyped@55dc209

So can you please alter the HelmetProvider type to have children as a prop?

...
export class HelmetProvider extends React.Component<React.PropsWithChildren<ProviderProps>> {
  static canUseDOM: boolean;
}
...

Thanks a lot 🌷

@andipaetzold
Copy link
Contributor Author

@bacali95 Good catch, @types/react@18 was released after I created the PR, so I missed the changes.

I am gonna update the PR to ensure it is compatible with both @types/react@17 and @types/react@18.

@andipaetzold andipaetzold changed the title feat: add React 18 to peer deps feat: support React 18 Apr 8, 2022
@andipaetzold
Copy link
Contributor Author

@bacali95 I just updated the PR

index.d.ts Outdated Show resolved Hide resolved
@bacali95
Copy link

bacali95 commented Apr 8, 2022

Hey @andipaetzold thanks for the quick fix 🌷

Hey @staylor can please review/approve this PR 🌷

@staylor
Copy link
Owner

staylor commented Apr 9, 2022

this needs to be rebased on your side, then I can merge it

@types/react v17 added the children prop automatically. This changed
with v18. Now, PropsWithChildren needs to be manually added.
@andipaetzold
Copy link
Contributor Author

@staylor done

@dquoctri
Copy link

I have a workaround to fix this issue by myself. I'm waiting for this PR.
Thank you all!

export interface HelmetProps {
...
children?: React.ReactNode;
}

interface ProviderProps {
context?: {};
children?: React.ReactNode;
}

@kimbaudi
Copy link

Thanks for this PR. Looking forward to the next release. 😄

@staylor staylor merged commit 24bb1b1 into staylor:main Apr 11, 2022
@andipaetzold
Copy link
Contributor Author

Thank you 🚀

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.

Update typedef to support React 18 Typedef changes Support React v18
5 participants