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

Children type should be ReactNode instead of ReactElement #268

Closed
erikbrgn opened this issue Oct 31, 2024 · 2 comments · Fixed by #270
Closed

Children type should be ReactNode instead of ReactElement #268

erikbrgn opened this issue Oct 31, 2024 · 2 comments · Fixed by #270
Assignees

Comments

@erikbrgn
Copy link

The type of the children prop in FlagsmithContextType doesn't align with the type normally used for children props which results in having to wrap an actual children prop used inside a FlagsmithProvider with a fragment to make it work without TS errors. Perhaps the type could be changed from ReactElement | ReactElement[] to simply ReactNode, which would be the same as the PropsWithChildren generic type provided by React.

// Example of error
type FeatureFlagProviderType = {
  serverState: IState;
};

export const FeatureFlagProvider = ({
  serverState,
  children, // ReactNode
}: PropsWithChildren<FeatureFlagProviderType>) => {
  const flagsmithInstance = useRef(createFlagsmithInstance());
  return (
    <FlagsmithProvider
      flagsmith={flagsmithInstance.current}
      serverState={serverState}
    >
      {children}
    </FlagsmithProvider>
  );
};

However, by wrapping children with a fragment or something else, there are no issues.

Perhaps I'm missing something, but is there any special reasoning behind using ReactElement | ReactElement[] for children?

@matthewelwell
Copy link
Contributor

Hi @erikbrgn , thanks for raising this. We'll take a look and confirm.

@kyle-ssg kyle-ssg linked a pull request Nov 6, 2024 that will close this issue
@kyle-ssg
Copy link
Member

This is now released under version 8.0.1 🚀

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 a pull request may close this issue.

3 participants