🦋 Changeset detected

Latest commit: f3c4c64

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f3c4c64:

Hmm, why is this error happening here?

➤ YN0000: [@emotion/react]: Process started
➤ YN0000: [@emotion/react]: dtslint@0.0.112
➤ YN0000: [@emotion/react]: Error: /home/runner/work/emotion/emotion/packages/react/types/tests.tsx:259:3
➤ YN0000: [@emotion/react]: ERROR: 259:3  expect  TypeScript@4.7 compile error: 
➤ YN0000: [@emotion/react]: 'OtherComponent' cannot be used as a JSX component.
➤ YN0000: [@emotion/react]:   Its type '{ <Tag extends ElementType<any>>(props: AllHTMLAttributes<HTMLInputElement> | ComponentPropsWithRef<Tag>, ref: any): Element; displayName?: string | undefined; }' is not a valid JSX element type.
➤ YN0000: [@emotion/react]:     Type '{ <Tag extends ElementType<any>>(props: AllHTMLAttributes<HTMLInputElement> | ComponentPropsWithRef<Tag>, ref: any): Element; displayName?: string | undefined; }' is not assignable to type '(props: any) => ReactElement<any, any> | null'.
➤ YN0000: [@emotion/react]:       Target signature provides too few arguments. Expected 2 or more, but got 1.
➤ YN0000: [@emotion/react]: 
➤ YN0000: [@emotion/react]:     at testTypesVersion (/home/runner/work/emotion/emotion/node_modules/@definitelytyped/dtslint/dist/index.js:194:15)
➤ YN0000: [@emotion/react]:     at async runTests (/home/runner/work/emotion/emotion/node_modules/@definitelytyped/dtslint/dist/index.js:170:13)
➤ YN0000: [@emotion/react]:     at async main (/home/runner/work/emotion/emotion/node_modules/@definitelytyped/dtslint/dist/index.js:[79](https://github.com/emotion-js/emotion/actions/runs/5172904991/jobs/9317682961?pr=3048#step:4:80):9)
➤ YN0000: [@emotion/react]: Process exited (exit code 1), completed in 20s 272ms

I don't know much about JS and TS, but it is hard to believe that this Pull Request change affects React.ElementType, so I thinking it was a @definitelytyped/dtslint issue and I updated to 0.0.115 and this error did not occur.
I don't know the details of why this problem no longer occurs with 0.0.115.

On the other hand, when I updated to the latest 0.0.163, I get the error everywhere. These seemed to be caused by existing type definitions that were now working fine.

Sorry, I see that it has fallen unchanged in CI... On my local, the problem remains at 0.0.112 and remains resolved at 0.0.115.

Do you know anything about this?(why is this error occurring in the first place, Etc.)

1. I think that this particular test case that is failing can be removed, this will revert~ Fix TypeScript support for custom components with non-standard declarations #2181 (cc @101arrowz) but that seems to be the correct thing to do. That PR was enabling something that isn't quite type-safe.
2. so why introducing the JSX.ElementType here is creating a problem in the first place? With ElementType in place the tagType is being checked against elementTypeConstraint to see if it's assignable to it, whereas previously only return types were compared by TS using checkJsxReturnAssignableToAppropriateBound (see source code here)
3. it happens so that we have @types/react@18.0.14 installed here and that defines type JSXElementConstructor<P> = ((props: P) => React.ReactElement<any, any> | null) | (new (props: P) => React.Component<any, any>) but the current @types/react@18.2.8 have a slightly different definition, see here.
4. We can update @types/react as part of this PR (it has no effect on our consumers anyway, locally it's just a dev dependency and we forward whatever definitions are installed in consuming projects), which would fix the caught issue. The point is that how @types/react got adjusted 2 weeks ago is a temporary~ thing (see the PR here) so the test that is failing right now can stop working at just any time in the future
5. we can double-check how those 2 JSXElementConstructor "perform" against this component in this TS playground. Why do I think that this definition isn't type-safe? This particular definition has a required second parameter in the signature of type any. This makes it somewhat OK since u forgo all the type safety with any anyway but if that would be unknown or just any other type then things could break badly. With JSX we are handing out functions to some runtime (in this case it's React) and that runtime is meant to call those functions on our behalf. But React can't call our functions with arbitrary extra arguments and yet our signatures might depend on them. In practice, this doesn't quite happen often and it's not a common problem but from the types PoV this is just wrong. It seems that TS has traditionally allowed this (see the playground here with the required string in the signature used as JSX element). The recent changes to JSX.ElementType will allow people to avoid such invalid combinations in the future (and already do it for cases with 3+ function parameters)

@Andarist Wow, I am very happy to see a fairly thorough explanation! Thank you very much!

I tried each of the things you taught me, but this time I just decided to delete the test case.

Once I updated @types/react, this dragged the test cases in packages/native to fail. Probably due to the relationship between @types/react and @types/react-native, but I thought it was too broad to do in this PR.

If the test in the emotion fails, I thought about changing the ElementType as you said, but since that was not the case, I think we should leave it as is for the time being.

Thank you for your kind review, I'm so very happy! is there anything else I need to do to get this PR merged?

Thanks for the PR, review and merge @naari3, @Andarist, @emmatown ! 🙌

Looking forward to trying this out in @emotion/react@11.11.1 after #3051 gets merged!