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

createPortal return type is not a valid JSX element #925

Closed
joshuaellis opened this issue Jan 11, 2021 · 7 comments · Fixed by #937
Closed

createPortal return type is not a valid JSX element #925

joshuaellis opened this issue Jan 11, 2021 · 7 comments · Fixed by #937

Comments

@joshuaellis
Copy link
Member

Relevant code or config:

An example of using createPortal:

function UseCameraScene() {
  return createPortal(<Object />, Scene)
}

The TS implimentation of createPortal from r3f library:

export declare function createPortal(children: React.ReactNode, containerInfo: any, implementation?: any, key?: any): {
    $$typeof: number | symbol;
    key: string | null;
    children: React.ReactNode;
    containerInfo: any;
    implementation: any;
};

What you did:

Tried to use createPortal in Drei's useCamera.stories.tsx as part of pmndrs/drei#223

What happened:

The following TS error was thrown:

'UseCameraScene' cannot be used as a JSX component.
  Its return type '{ $$typeof: number | symbol; key: string | null; children: any; containerInfo: any; implementation: any; }' is not a valid JSX element.
    Type '{ $$typeof: number | symbol; key: string | null; children: any; containerInfo: any; implementation: any; }' is missing the following properties from type 'Element': type, props

Problem description:

createPortal should return a valid JSX element since it needs to be returned in the react component.

Suggested solution:

I'm not sure yet, it probably needs someone who has more knowledge of r3f than I. I'll update if I have one.

@drcmda
Copy link
Member

drcmda commented Jan 12, 2021

just casting it maybe? i dont know TS enough to know if that's the way but the implementation is technically correct and works.

@joshuaellis
Copy link
Member Author

Casting like so:

return createPortal(<Object />, Scene) as JSX.Element

fails with the same error. However, casting like so:

return (createPortal(<Object />, Scene) as unknown) as JSX.Element

will pass. Although I don't think this is quite the correct way. Is there anyone on the team who's particularly skilled in TS? I can look some more today as well.

@drcmda
Copy link
Member

drcmda commented Jan 12, 2021

I think for TS in general yes, but this goes into reconciler internals so im not sure if anyone really has had experience with that. the react projects (dom, art, native) are all plain js, so we can't even look there. alternatively we could look at konva and ink, maybe they have found proper types.

@joshuaellis
Copy link
Member Author

react has types, but it's in the definitely typed repo. And there is a type for Portal but it doesn't match the implimentation... 🤔 But konova and ink are also good leads. 💪🏼

@drcmda
Copy link
Member

drcmda commented Jan 12, 2021

maybe the r3f implementation is outdated, i copied it directly from a suggestion from dan. ts aside, it still works fine though - not sure what the problem could be.

@joshuaellis
Copy link
Member Author

An update: It look's like it is a problem with @types/react anyway – DefinitelyTyped/DefinitelyTyped#20942, although their solution doesn't really work...

@joshuaellis
Copy link
Member Author

joshuaellis commented Jan 13, 2021

If createPortal's return type is React.ReactNode this seems to remove the errors:

export function createPortal(
  children: React.ReactNode,
  containerInfo: any,
  implementation?: any,
  key: any = null
): React.ReactNode {
  if (!containerInfo.__objects) containerInfo.__objects = []
  return {
    $$typeof: REACT_PORTAL_TYPE,
    key: key == null ? null : '' + key,
    children,
    containerInfo,
    implementation,
  }
}

@stephencorwin do forbode any issues with this impl?

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.

2 participants