stencil bug: Incorrect TypeScript typings for JSX

Prerequisites

[X] I have read the Contributing Guidelines.
[X] I agree to follow the Code of Conduct.
[X] I have searched for existing issues that already report this problem, without success.

Stencil Version

4.12.0

Current Behavior

It appears that the TypeScript definitions for JSX support in Stencil are not completely correct, and are causing issue when used with third-party tools (like @typescript-eslint)

Expected Behavior

JSX.Element type is resolved correctly

System Info

System: node 20.11.0
    Platform: darwin (23.2.0)
   CPU Model: Apple M1 Pro (10 cpus)
    Compiler: /Users/mak13180/site/esri/arcgis-web-components/node_modules/@stencil/core/compiler/stencil.js
       Build: 1705333241
     Stencil: 4.10.0 🍪
  TypeScript: 5.3.3
      Rollup: 2.42.3
      Parse5: 7.1.2
      jQuery: 4.0.0-pre
      Terser: 5.26.0

Steps to Reproduce

I made a small reproduction case based on the JSX typings in Stencil: source

See the error on line 16 Further, see that see that typescript compiler resolves the type as "error":

https://github.com/ionic-team/stencil/assets/40512816/0957aa3a-c159-4779-b133-85389156b0a8

This is because TypeScript is hardcoded to look for Element type/interface in the JSX namespace In the above demo, uncomment line 8 (not present in Stencil's typings), and see how the typescript compiler now correctly resolves the JSX type, and also the typescript-eslint error is resolved

Code Reproduction URL

Initial problem: ESLint Playground - Example 1: "Unsafe return of an any typed value."
After setting type Element = VNode in JSX namespace: ESLint Playground - Example 2: "'Fragment' cannot be used as a JSX component. Its return type 'VNode | VNode[]' is not a valid JSX element."
Stencil Component Starter Reproduction

Additional Information

For reference, here are examples of Elements interface/type being included in the JSX element in other popular libraries:

React - https://github.com/DefinitelyTyped/DefinitelyTyped/blob/6d0e970598392a8c1c62019150e4f197fadf301c/types/react/jsx-runtime.d.ts#L5
PReact - https://github.com/preactjs/preact/blob/49433b3a16a629db3a9a39df3f9f81ff703078bb/src/jsx.d.ts#L41
dom-expressions (used by SolidJS and others) - https://github.com/ryansolid/dom-expressions/blob/eb9a6bcf640b04c1d1c889d9983914a2776093fe/packages/dom-expressions/src/jsx.d.ts#L12
Qwik - https://github.com/BuilderIO/qwik/blob/408576e8a2e1d11fabf030f37b5941c0a3274669/packages/qwik/src/core/render/jsx/types/jsx-qwik.ts#L7

After debugging typescript-eslint and reading TypeScript source code, it appears that TypeScript is hardcoded to look for JSX.Element type/interface, rather than look at the return type of the jsxFactory:

    function getJsxElementTypeAt(location: Node): Type {
        return getJsxType(JsxNames.Element, location);
    }

(TS 3.7 source)

https://github.com/ionic-team/stencil/assets/40512816/4b5f4874-7975-4320-bd5f-d862af964272

Jan 31 '24 04:01 maxpatiiuk

These types appear incorrect too:

export declare const Host: FunctionalComponent<HostAttributes>;
export declare const Fragment: FunctionalComponent<{}>;

because FunctionalComponent is defined as returning VNode | VNode[], but TypeScript expects Fragment to be equal to JSX.Element (thus just VNode, not VNode | VNode[])

export interface FunctionalComponent<T = {}> {
    (props: T, children: VNode[], utils: FunctionalUtilities): VNode | VNode[];
}

otherwise, the user of Stencil would be forced to specify return type as VNode | VNode[] everywhere in there application (i.e as render() return type)

and you get this TypeScript error:

'Host' cannot be used as a JSX component.
  Its return type 'VNode | VNode[]' is not a valid JSX element.ts(2786)

/// and:

'Fragment' cannot be used as a JSX component.
  Its return type 'VNode | VNode[]' is not a valid JSX element.
    Type 'VNode[]' is missing the following properties from type 'VNode': $flags$, $tag$, $elm$, $text$, $children$ts(2786)

something like this fixes it:

export declare const Host: (props:HostAttributes)=>VNode;
export declare const Fragment: (props: {})=> VNode;

Jan 31 '24 04:01 maxpatiiuk

@maxpatiiuk Thanks! Someone will take a look at this in a bit.

Can you do me a favor and split https://github.com/ionic-team/stencil/issues/5306#issuecomment-1918341335 into its own issue? I'd like to make sure we're not trying to understand two different Stencil issues in the context of one GitHub issue. Thanks!

Jan 31 '24 19:01 rwaskiewicz

Can you do me a favor and split https://github.com/ionic-team/stencil/issues/5306#issuecomment-1918341335 into its own issue? I'd like to make sure we're not trying to understand two different Stencil issues in the context of one GitHub issue. Thanks!

unfortunately, these fixes have to be in the same PR.

reason:

The problem is that adding the type Element = VNode; uncoverts the typing issues with Fragment/Host return type, causing TypeScript errors:

'Fragment' cannot be used as a JSX component.
  Its return type 'VNode | VNode[]' is not a valid JSX element.
    Type 'VNode[]' is missing the following properties from type 'VNode': $flags$, $tag$, $elm$, $text$, $children$ts(2786)

basically, we defined that JSX.Element is VNode, but return type of Fragment is not VNode but VNode | VNode[], so TypeScript complains. The other solution is to define type Element = VNode | VNode[]; but that is a bad idea (would force users of Stencil to always type the returns of their render() functions as VNode | VNode[] instead of just VNode)

(this is a limitation of TypeScript, see https://github.com/DefinitelyTyped/DefinitelyTyped/issues/20544 and https://github.com/microsoft/TypeScript/issues/21699)

so, while I could separate the PRs, they would have to be merged at the same time to avoid issues

Jan 31 '24 20:01 maxpatiiuk

Hey! So while that context is helpful, what I was looking for a actually a separate issue, rather than PR. Specifically, I am looking for steps to reproduce the statements that Host and FunctionalComponent are incorrect. I'd like to understand where/how the errors you're showing in this comment are coming from (be it the output of stencil build, in an editor, a combination of both, etc.).

Prior to your second comment, it wasn't clear these were related. I still haven't dug in, but if you can let me know where how you're seeing these errors after applying that type Element = VNode change, that'd be a big help 👍

Jan 31 '24 20:01 rwaskiewicz

Ah, thank you for the clarification - I misread your comment Opened an issue, with the smallest reproducible code sample and a suggested fix - https://github.com/ionic-team/stencil/issues/5311

Feb 01 '24 01:02 maxpatiiuk

Thanks! In #5311 I asked for a reproduction case that doesn't use the ESLint playground. I don't think that'll be necessary here, as I think I can use the same one for this issue (I may update the Issue Summary to link to it when it's available). If you don't think I could (reuse the reproduction case), please LMK!

Feb 01 '24 14:02 rwaskiewicz

I've merged https://github.com/ionic-team/stencil/issues/5311 into this issue now that I understand the problem space a bit better.

For folks running into this, the current plan is to revisit the the PR that fixes this as a part of Stencil v5. In the interim, I suggest adding an any return type to your render functions, as that appears to appease the ESLint rule no-unsafe-return and aligns with the currently typed return value of render

Feb 02 '24 14:02 rwaskiewicz

Thanks - this sounds good.

We are not allowed to use any in our codebase, but there is a nice workaround:

Create a declarations.d.ts file like this:

import type { VNode } from "@stencil/core";
import type { FunctionalUtilities } from "@stencil/core/internal";

declare module "@stencil/core" {
  export namespace h.JSX {
    type Element = VNode;
  }

  // eslint-disable-next-line @typescript-eslint/ban-types
  export interface FunctionalComponent<T = {}> {
    // eslint-disable-next-line @typescript-eslint/prefer-function-type
    (props: T, children: VNode[], utils: FunctionalUtilities): VNode;
  }
}

(even though the problem is with Host/Fragment types, typescript doesn't let us override those due to a Subsequent variable declarations must have the same type. error, so I am overriding FunctionalComponent return type instead - this might cause issues in some codebases, but worked fine in our's)

Then in your tsconfig.json, in the "files" array, add this file

{
  // ...
  "files": [
    "./declarations.d.ts",
  ],
  // ...
}

Feb 02 '24 15:02 maxpatiiuk

@maxpatiiuk Thanks for your workaround! Unfortunately type inference of props in FunctionalComponents stops working with this.

The simple example below is taken form the official docs:

import { FunctionalComponent, h } from '@stencil/core';

interface HelloProps {
  name: string;
}

export const Hello: FunctionalComponent<HelloProps> = ({ name }) => (
  <h1>Hello, {name}!</h1>
);

name should be a string:

I don't understand how changing the return type of the FuntionalComponent Interface (VNode|VNode[]->VNode) results in this behavior.

Any ideas on how to get my types back?

Apr 22 '24 11:04 cl10k

facing the same issue which is related to this as well -> https://forum.ionicframework.com/t/adding-ionic-components-to-custom-stencil-components/223091/2

Trying to use @stencil/core and load it via an app.ts file. This works well until I try to build a react version of the library. Generated typings for The @ionic/core components fail. export const IonAccordion = /*@__PURE__*/createReactComponent<JSX.IonAccordion, HTMLIonAccordionElement>('ion-accordion'); export const IonAccordionGroup = /*@__PURE__*/createReactComponent<JSX.IonAccordionGroup, HTMLIonAccordionGroupElement>('ion-accordion-group'); export const IonActionSheet = /*@__PURE__*/createReactComponent<JSX.IonActionSheet, HTMLIonActionSheetElement>('ion-action-sheet'); ... exports like JSX.IonAccordion fail which causes the react lib build to fail. Any ideas on how to fix this?

May 07 '24 15:05 andluchian