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

bug(jsx): Fragment and Host return types are incorrect, causing TypeScript errors #5311

Closed
3 tasks done
maxpatiiuk opened this issue Feb 1, 2024 · 6 comments
Closed
3 tasks done

Comments

@maxpatiiuk
Copy link

maxpatiiuk commented Feb 1, 2024

Prerequisites

Stencil Version

4.12.0

Current Behavior

After #5306 is resolved, typescript is able to correctly resolve JSX.Element type (before that, it's implicitly any)
That uncovers another error in the typings - Fragment and Host are typed as returning VNode | VNode[], but should return VNode

This manifests as the following TypeScript errors:

'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)

Expected Behavior

The return type should be VNode to resolve the errors

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

  1. Open this code
  2. See the TypeScript error
  3. See that changing Fragment and Host return type to VNode fixes the issues

Code Reproduction URL

https://typescript-eslint.io/play/#ts=5.3.3&showAST=types&fileType=.tsx&code=KYDwDg9gTgLgBASwHY2FAZgQwMbDgNQDkIATPAbzgGcEBbMAGwXQWBIDFoAhKYANwQwAngC44MKAFdgAbjgBfAFCLQkWIhRosuOO0lJsMBBCSYGAVSNMjwKnEoB6AFRO4EWoNQk46aHABGvALCcE4OCirg0PDIqBg4eAASEFQwAIIwEgj%2Bkqh2ji5uHplsPn6B-IJCoeFKqtEacdp4egZGJmYAwu6QSMAoADwAKnAAvPbyAHz2inBwABRgUBBgVGJDADRw2AAWCAwkvEhiRKTAANoAulu5%2B2u6%2BobGphZWgqxUAJQnxGRwAD4EX4XS4yRRKZQOcJDHZJFLwTBIbzsKCYADmtH68GEYGQaLsmF4Gmw0F4hhEkTU8DI2AYhLwJKQqTgyVSYlaTw6DG69BMWIGrPSmSg2VytkmYPq6hpdKJjOZKPRmJQ7Me7RePN6-PIUzBULgAGVgHgYLCqHg0MsoGtFOg1c9xLYYPNvkCzjM5swFgBGMajcbez5wXgwSRQJBwAXwuAOCWzYPAUPhyOKjFYmNxiGUhoy%2BlwUyYqhgBJwHYenz2kyl%2BbmhhifQAayQEAA7kgtiRMDBMGIkJIGAxXacyGC5gXbMWdAApA0ADXLc31MLwmH8ED4FqgVrsJgY1UwYDAwEJcEw6DipcyqxEULRgh2kn8ADoSbQHM8ENgALSoTBv1L9Ng%2BzvlQVDSFQDgAKwAMwAAwAGzxou4QIHYLAgGwWz%2BMA2CYJI5puKaaAtqheBDEIR4GtgIpgDEdj6KuDAmhACZUBADAbkhMbhDOs5PgAokxyrYhRwBbIi3g0ExKB7j4ZgMHY-g4A24gsQARIiQhqVxOJ4IJwDCWMbojvGShZkusJwJafioT4CAYd4-jVLQmANniLLRhJuiommKAJkmEa6Yo%2BrYcZeCokRUDiDsiJhQCYVXBSUrUjhsoMiYzKCgATGIizLNeHmpBkWQ5HkQajNMw6yNm0qpXm8rwKmwk5QsSwrPcOrlZVwJgnabQOnkMBZS6PzuuQ8ZevMvr%2BgGQYhmGEZRqkWUZqO-kLSmPnNat4JAA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1tieQEMAZolrRE%2BWNCbooiaNA7RI4MAF8QKoA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3QAacDUgArEgA8KkTOgCGzSJJpQ58gGLq8sTAE8KUHpCkBfENaA&tokens=false

Additional Information

Related issue: #5306

@rwaskiewicz
Copy link
Member

Hey @maxpatiiuk 👋

Thanks for the second issue! Would it be possible to give us a reproduction case that doesn't use the TS ESLint playground? It makes it a bit difficult to evaluate the changes proposed in #5307.

@rwaskiewicz rwaskiewicz added ionitron: needs reproduction This PR or Issue does not have a reproduction case URL and removed triage labels Feb 1, 2024
Copy link

ionitron-bot bot commented Feb 1, 2024

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Stencil starter component library and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

@maxpatiiuk
Copy link
Author

That TS ESLint playground is sufficient to reproduce the issue, and it shows the internals of TypeScript compiler with additional information.
Still, if you prefer a stencil repository, here is a reproduction case: https://github.com/maxpatiiuk/stencil-issue-5311-reproduction

@rwaskiewicz
Copy link
Member

Hey @maxpatiiuk,

I'm unable to reproduce this issue based on the instructions you've provided.

To be clear, I'm asking for a reproduction case so that we can evaluate the changes you made in #5307 with the actual Stencil compiler, rather than something that replicates it in a playground. This is why I asked for a reproduction case - I'm unable to compile Stencil and get it "into" the ESBuild playground.

As a part of that evaluation, I'm looking to replicate the error in the reproduction case you provided. From the instructions you've provided, it looks like you're asking me to look at src/declarations.d.ts, but it's unclear how I can replicate these errors in the first place. It's unclear to me if this is:

  • an eslint error (if so, how can I replicate it?)
  • a build error
  • errors in an editor (if so, which?)

While I think it's the first option based on the context of this issue, I'd greatly appreciate it if you can provide step by step instructions here for the team to be able to take a look.

Thanks!

@maxpatiiuk
Copy link
Author

I'm unable to reproduce this issue based on the instructions you've provided.

See the updated README with detailed steps for both issues: https://github.com/maxpatiiuk/stencil-issue-5311-reproduction#readme

I am able to reproduce issues the ESLint and TypeScript issue both in the command line and in VS Code on a fresh clone of that repository

@rwaskiewicz
Copy link
Member

Thanks! I've merged this into #5306 now that I understand the problem space a bit better. Per my comment here I'm going to close this out.

@rwaskiewicz rwaskiewicz closed this as not planned Won't fix, can't repro, duplicate, stale Feb 2, 2024
@rwaskiewicz rwaskiewicz removed the ionitron: needs reproduction This PR or Issue does not have a reproduction case URL label Feb 2, 2024
@rwaskiewicz rwaskiewicz removed their assignment Feb 2, 2024
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

No branches or pull requests

2 participants