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

Playground: dependencies for jsxImportSource are not installed #1427

Closed
trusktr opened this issue Dec 16, 2020 · 9 comments
Closed

Playground: dependencies for jsxImportSource are not installed #1427

trusktr opened this issue Dec 16, 2020 · 9 comments
Labels
Playground ATA Holes in the current type acquisition Playground Issues that affect the Playground

Comments

@trusktr
Copy link
Contributor

trusktr commented Dec 16, 2020

TS team decided to use /* @fooImportSource */ syntax for importing JSX types (otherwise if JSX types are imported with import syntax then the JSX expressions do not see the JSX namespace), but the playground is unaware that it needs to import the dependencies for that sort of syntax:

/* @jsxImportSource solid-js */
const d = <div /> // Error: interface JSX.IntrinsicElements is not defined, because playground does not install solid-js

playground example

If we could instead rely on import for importing JSX types (as would be intuitive and aligned with EcmasScript standards and conventions, instead of having to use new @fooImportSource syntax), the playground example (which will install solid-js based on the import statement) would just work already.

@trusktr trusktr added the Playground Issues that affect the Playground label Dec 16, 2020
@trusktr
Copy link
Contributor Author

trusktr commented Dec 16, 2020

I also tried placing foo-runtime.d.ts in a package, and then importing it like this:

/* @fooImportSource foo-package */

hoping it would import the FOO namespace from foo-package/foo-runtime.d.ts but that didn't work.

I think we should stick with ES Module semantics anyway, because that's the standard way to import everything in JS world (and hence TS world). No idea why we want to import things with a new @fooImportSource comment syntax.

@orta
Copy link
Contributor

orta commented Dec 16, 2020

The compile-time semantics are built to replicate the source of truth for JSX parsing (aka the babel plugin) - but sure, I'd accept a PR adding the module in fooImportSource to the ATA in the playground 👍🏻

@trusktr
Copy link
Contributor Author

trusktr commented Dec 16, 2020

@orta My last comment was intended as a joke, but the actual attempt to use /* @jsxImportSource package-name */ in the OP is what is not working (try the playground link).

(Just kidding about supporting arbitrary @anythingImportSource syntax).

@trusktr
Copy link
Contributor Author

trusktr commented Dec 17, 2020

TypeScript is doing the wrong thing with JSX, in my opinion. We should be able to use import {JSX} from 'anywhere' syntax instead of @jsxImportSource anywhere syntax, and it should just work but it doesn't.

@trusktr
Copy link
Contributor Author

trusktr commented Dec 17, 2020

@orta One more example to make the bug clear:

https://www.typescriptlang.org/play?jsx=1#code/PQKhAIAECsGcA8CSBbADgewE4BcDK6BXTAYwFNxZ0AbASwBMBaOcEYAKBrS23AG8ApXAA0AvuABmmdMnABySrUZxZAbjZtsAT1Tlx6dOAC84QUIB0iAHbZMNS7BrEAolVLJS12OGDBw6ANYANOCWBqSYUpjBpuA0XnYUxOg64AAW4aRm6nSkxFQAhpjkyOh0BK5yCvRMsLJ8bOAh+e6wqPlkJsL1jY122OHi7eRWNnYOzq7unt09jbJ66Aykkx7YsgBcfAublgTIAEbhIg09x43Hx2xJ9jzLRuAAFCc+4ABCBDwxcSHoPAmwSRS6SKAH4TgAeBZLFbWAB8AAlllQDAB3LBUOjg4BQ5ZuVawtgAShUQA

Two things:

  1. playground does not correctly discover solid-js/jsx-runtime, thus it has an error
  2. If TypeScript's JSX (TSX) had a better design not guided by React (i.e. none of this jsxImportSource with hard-coded /jsx-runtime.* file path stuff), and instead allowed it to work with the intuitive import {JSX} from '...' syntax, then
    a. the playground example would just work, just as anyone familiar with ES Module syntax would expect, because it would work just the same as with all existing imported code.
    b. and the code would be DRY instead of WET, because as you can see in the example JSX has to be imported twice for things to work,
    • once for non-JSX code within the file (using import {JSX} from '...' syntax)
    • and once for JSX code within the same file (using @jsxImportSource ... syntax)

@orta
Copy link
Contributor

orta commented Dec 17, 2020

The compiler follows the JSX babel implementation, so if that changes so will we.
The website issues isn't the place for discussing that.

That said this is the website repo, and I'm still OK with what I said earlier. I'd accept a PR for where ATA would handle this case but given that most examples would import the module for its API anywau, it'd get picked up by that.

@trusktr
Copy link
Contributor Author

trusktr commented Jan 19, 2021

but given that most examples would import the module for its API

Not sure if by that you mean import {JSX}, but that doesn't work for JSX markup (this issue).

I made another example, that shows that even if solid-js exists and I can import its APIs, @jsxImportSource still doesn't see the package: playground.

This JSX stuff is difficult to learn, and by it not working in playground may give users a false sense of that approach being wrong.

@trusktr
Copy link
Contributor Author

trusktr commented Aug 14, 2022

Looks like the problem has been partially fixed, and now my previous comment's example works. @jsxImportSource will work so long as an import statement also imports the package, that way the package will be installed locally by the import statement and hence the @jsxImportSource machinery will see the package.

playground example

If you comment that line out (and refresh), then the @jsxImportSource stops working:

playground example

The separate import is still required for @jsxImportSource to work... but it works now! 🎉

@typescript-bot
Copy link
Collaborator

Hello! As per #2804, we are automatically closing all open issues. Please see #2804 for a description of what issues and PRs can be accepted going forward.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Playground ATA Holes in the current type acquisition Playground Issues that affect the Playground
Projects
None yet
Development

No branches or pull requests

3 participants