-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Add emit support for jsx/jsxs experimental jsx runtime api #39199
Conversation
@typescript-bot pack this |
Heya @weswigham, I've started to run the tarball bundle task on this PR at 330af8d. You can monitor the build here. |
Hey @weswigham, I've packed this into an installable tgz. You can install it for testing by referencing it in your
and then running There is also a playground for this build. |
This is an interesting piece of information for me - or it might be. Could you clarify for me if this works on per-config basis (or per-file when using Right now JSX "pollutes" the global namespace and AFAIK what I'm talking about is not possible. |
We actually already support localized JSX namespaces (and therefore loading multiple into a project (and using them via pragmas)) - |
Huh, that's great to hear - I will have to experiment with how it behaves to get a better feeling of how it works in various situations. 👍 Do you happen to know when the support for this was added? If With this PR it would be great to be able to customize the |
Same time as support for per-file
Pretty bad, I think. There's a lot of |
This is awesome - I have been searching for a way to define such local JSX namespace for a while and wasn't aware that this is already implemented! I've played a little bit with it and it seems to work pretty well - I only have one caveat with it: it doesn't support EDIT:// I've tracked down the PR adding support for this and it seems that what I'm after can actually be done by defining a factory function and a namespace with the same name. I had no idea that this is possible. In a way, my problem is solved with this - but I kinda still believe that looking up for a module-level |
@typescript-bot pack this |
Would it be possible for you to support a new option (existing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me.
My admittedly naive feelings on whether a tsx file should be treated as a module in this context is that it should. I don't know how you can't ever get a JS file which represents a script with these new imports.
Thanks this worked. I figured vscode was using an current stable version and not next 😊 |
@weswigham The fallback to if using |
Note #41146 is critical to fix not only for React-like libs, but for React itself too, since it would otherwise lead to a non-existent reference being emitted. |
I have started looking into fixing this - doesn't look overly complicated but I haven't contributed much to the codebase, so we'll see how it goes. |
@gaearon I've implemented this locally already - however, I've discovered that I will have to adjust the implementation further. I'll probably wrap this up tomorrow, but I've wanted to report back here, especially because of the obstacle I've encountered. TS compiler currently assumes that all imports that have to inserted comes from a single entry and thus after my changes the |
I don’t think anyone is arguing that this isn’t confusing for the implementors. :-) In particular because it wasn’t clearly specified in the RFC. That’s our fault. But I think talking about whether this is confusing or not for the implementors doesn’t move the discussion forward. I’m not sure I understand the goal of continuously bringing this up. It confused me too, I dug out the reasoning and wrote it up in the linked thread. I hope it’s helpful. I’m also happy to see alternative suggestions in the React issue thread. |
Re: confusion — if this is the criticism of the rollout and communication, I fully agree we could do a better job about this case. It is indeed very subtle (and I only understood it yesterday myself). I’m not sure if you’re just communicating frustration about this or if you want to change this to a different solution. Regardless this should probably be discussed on the React repository instead. |
VScode shows me |
|
typescript has support new jsx transformation, see microsoft/TypeScript#39199
typescript has support new jsx transformation, see microsoft/TypeScript#39199
@weswigham Can we sync next time if this requires work on |
If you are referring to the types for the
I've also commented in that DefinitelyTyped PR. |
This is the reason I'm getting this error?
|
It seems to me. Changes like DefinitelyTyped/DefinitelyTyped#49701 fix the issue for me so it seems like the new compiler options need new releases of |
can we use this with TypeScript3.9? |
No, it's a 4.1 feature. |
Was wondering: is there a way to emit preserved JSX, but using a .js extension instead of .jsx? On Docusaurus we use Wouldn't it make sense to introduce a more generic "preserve-js" option or something? (exact same behavior as "react-native", just an alias) |
React Native was a weird (admittedly frustrating) case where Metro insisted on not accepting any file type other than |
Docusaurus can use .jsx but we'd prefer not to. We use tsc to output clean React theme components that can be "swizzled": the React theme components are then copied to the user's source code for shadowing the original implementation so that users can override the default UI we provide. Using We can probably rename the extension at copy time but it would look cleaner if there was an official way to output js instead of jsx. Not a big deal though, we have 2 possible workarounds ("react-native" or changing the extension ourselves) |
This adds two new options for the
jsx
compiler option -react-jsx
, andreact-jsxdev
. They use thejsx
andjsxDEV
constructors and thereact/jsx-runtime
andreact/jsx-dev-runtime
entrypoints, respectively. The primary differences, as noted in the linked issue, are thatchildren
are passed as part of theprops
object, and thekey
is passed as a separate argument. (ThejsxDEV
constructor further takes some debug data.) In addition, this adds ajsxImportSource
compiler option (and@jsxImportSource
file pragma) to control the root specifier the jsx runtime is imported from (which defaults toreact
). In it's current state, I believe this is usable, but there are some known flaws:jsxImportSource
module and use it as the source for the JSX namespace automatically. Presently you need to include it in the program yourself (ie, with atypes
directive or compiler option). This is moreso a TODO for myself, since I don't think there's any question to what we should do (implicitly include the package or@types
version in the program). This is more interesting for thejsxImportSource
pragma, which, for all intents and purposes, is animport
statement. We have a lot of services for real import statements - should they be applied to the pragma, where possible?jsxRuntime
pragma (with valuesclassic
orautomatic
) to swap between the old emit and the new one. This is supportable; however currently I only key our emit off thejsx
compiler option, and the presence of thejsxImportSource
pragma.createElement
(when akey
prop follows a spread) - this is a kind of soft deprecation; however it still implicitly imports the jsx runtime and pulls increateElement
. This PR does not yet do this, as, awkwardly enough, the modulecreateElement
is supposed to come from is different from thejsx-runtime
module entrypoint. I could make this work, but I think this is something we should provide some feedback on - so long as there's still acreateElement
fallback required in the API, why can't it be exported by the same entrypoint exportingjsx
orjsxDEV
?jsx
modes, the presence of a jsx tag imply module-ness?Fixes #34547
This is currently experimental in babel, and AFAIK not yet supported in a stable version of
react
; as such, we should probably continue to experiment with it for awhile before merging it into a stable release.TL;DR
When
jsx: react-jsx
(andtarget: esnext
) is set,compiles to