Make JSX pragmas override the configured runtime mode

[Andarist]

Feature Request

I'm basically requesting for what has been proposed by @nicolo-ribaudo here below the original PR introducing the new runtimes.

• I would like to work on this feature!

Is your feature request related to a problem?

Currently, there is no way to override the configured runtime with JSX pragmas and I think it should be possible, they are mutually exclusive and pragmas should not only be used to configure the current runtime but they should also have the power to reconfigure which runtime is being used. This solution seems to be the most robust one from the alternatives - allowing the most flexibility.

How this has been shipped is causing problems, especially in 0-config tools like CRA. They detect which runtime should be used by inspecting the installed version of React (which is IMHO fine) and this suddenly causes errors to be thrown when old @jsx pragma is already used in the code. It unnecessarily bounds together upgrades of projects like CRA and Emotion (the latter is @jsx-based).

Note: this already works like this in TS 4.1 betas (at least to some extent, need to test more combinations of pragmas and options), although there are still some issues with the overall implementation there, like microsoft/TypeScript#41146 and some warnings/errors for conflicting pragmas should be introduced. It shows though that they haven't thought about the need to restrict this which is what I'm asking for here.

Documentation, Adoption, Migration Strategy

Right now we have 3 situations that I'd like to make changes to:

1. runtime: classic (default) + @jsxImportSource

• actual: no error, no warning, the pragma is silently ignored
• expected: new transform being used

2. runtime: automatic + @jsx

• actual: error is being thrown
• expected: old transform being used

3. @jsx + @jsxImportSource

• actual: it depends on the runtime used, so it's either using classic runtime, ignoring the new pragma or an error is being thrown
• expected: make it a console.error, I believe it should use the new transform though (so it would change the current behavior) unless you consider this being a breaking change.
• expected in Babel 8: throw an error

I believe that the proposed solutions for each combination are not breaking changes.

cc @gaearon @lunaruan

[Andarist]

@gaearon @lunaruan @sebmarkbage any problems that you foresee with this? I would like to get into adjusting the transforms asap if I only get a green light on this one

[gaearon]

this suddenly causes errors to be thrown when old @jsx pragma is already used in the code.

What errors?

It unnecessarily bounds together upgrades of projects like CRA and Emotion (the latter is @jsx-based).

I talked to Seb and he actually suggested the opposite. To decouple the upgrades, the solution could be to specify both pragmas in the transitional period. Then drop the old one when you upgrade the tools. This could be a simple codemod.

[gaearon]

The downside of what you're proposing is that it's impossible to write a file that would work with both old and new transforms. E.g. if you run an internal company npm registry and you don't compile code until it's used in the app. If you have two apps with different CRA versions, with the proposed approach it would be impossible to write a component using Emotion correctly. But if the recommendation instead is to use both pragmas (when you want to support both tool versions), it works.

[Andarist]

What errors?

SyntaxError: /test__/index.js: pragma and pragmaFrag cannot be set when runtime is automatic.
> 1 | /** @jsx jsx */
    | ^
  2 | 
  3 | <div key="key" {...props} />;

I talked to Seb and he actually suggested the opposite. To decouple the upgrades, the solution could be to specify both pragmas in the transitional period.

Not sure if I fully understand the proposal - what would be an effect of both pragmas? Also - not sure if you got right the problem I'm describing so I'll try to rephrase - it's not a problem that one has to stick to one of the pragmas or smth but rather that if we don't make any changes to how this is implemented right now and if CRA sticks to configuring runtime: automatic based on the auto-detection (with which alone I have no problem with, I think the auto-detection there makes sense) then as soon as one upgrades their CRA version to the one supporting automatic runtime their Emotion-based projects will stop working because of the error I'm mentioning at the top of this comment.

EDIT:// Sorry - I'm in a hurry and just have digested your comments in full. Still not sure if you were implying that currently the upgrades are not coupled or just if my proposal doesn't decouple them enough. I guess maybe the latter - but was not obvious for me at first.

E.g. if you run an internal company npm registry and you don't compile code until it's used in the app. If you have two apps with different CRA versions, with the proposed approach it would be impossible to write a component using Emotion correctly.

IMHO this use case seems very very specific - not applicable to the majority of users. But sure - if u really want to support this then I would be OK with it, everything is better than throwing an error. My proposal would be for @jsxImportSource to simply override specificed @jsx and @jsxFrag.

[gaearon]

SyntaxError: /test__/index.js: pragma and pragmaFrag cannot be set when runtime is automatic

Oh I see. I forgot that there is an error on this. I think it would be fine to relax this and have it be ignored. This allows the transitional period where the same source file can be interpreted by either version as long as both pragmas are present. It is up to you, however, if this is valuable. We can always relax it later in a minor release too.

if we don't make any changes to how this is implemented right now and if CRA sticks to configuring runtime: automatic based on the auto-detection (with which alone I have no problem with, I think the auto-detection there makes sense) then as soon as one upgrades their CRA version to the one supporting automatic runtime their Emotion-based projects will stop working because of the error I'm mentioning at the top of this comment.

It will stop working until they replace the old pragma with the new one, right? In this case I argue that this is acceptable. People putting /* @jsx ... */ into every file know that they're opting into something special. I think it's reasonable that the "default" migration path is seamless but when you do customizations like this, upgrading might involve an extra step. I don't think Emotion users having to run a single Find-and-Replace operation in their project is the overriding concern here. It definitely needs to be communicated (e.g. wherever this technique is described in Emotion docs, or on StackOverflow), but once you do it, you no longer have the problem. Additionally, I think tools like CRA should provide an opt-out for the folks who use the new transform. I want to emphasize that at least in CRA's case, the new transform is bundled with a major version change, so people do expect that upgrading would need some manual steps.

[nicolo-ribaudo]

WDYT about never throwing but console.warn() if there is a pragma that is ignored by the used runtime? By doing so Dan's use case is satisfied (people using that two-modes setup can just ignore the warning), but we hint users that they are using the wrong pragma, since usually it would be a mistake.

[Andarist]

It will stop working until they replace the old pragma with the new one, right? In this case I argue that this is acceptable.

Yes, but they also need to upgrade Emotion to the latest v10 (we have to ship the new JSX factories after all( which shouldn't be a problem as it should be fully compatible but ideally this wouldn't be a required step - dependency upgrades are always "risky" (semver being human-driven and all). So I believe that if we could avoid this then it would be a preferable situation because upgrading things gradually rather than multiple things at once is usually a slower but safer approach

People putting /* @jsx ... */ into every file know that they're opting into something special

Special - but supported. There are probably a lot of "special" things handled automatically in CRA as webpack-based tool deviates from strict browser semantics - you can import assets 😱 ! And for newcomers, this doesn't really sound like anything different. From what I understand the CRA has been created both as a newcomer-friendly 0-config tool and a reusable boilerplate for others - I would argue that the current state of things just isn't that newcomer-friendly. And that's kinda my whole argument - because many people will just jump into upgrading both at once etc and I personally would not be affected by a mismatch like this.

It definitely needs to be communicated (e.g. wherever this technique is described in Emotion docs, or on StackOverflow), but once you do it, you no longer have the problem.

We can't update resources out of our control (blog posts, videos, whatever) and people using such with the freshly installed CRA (when the new major ships) would still have a problem that can be avoided.

WDYT about never throwing but console.warn() if there is a pragma that is ignored by the used runtime?

Sure, that's a possibility - although this could warn quite a bit for larger codebases wanting to have both pragmas at once.

---

I think the bottom line is that all parties involved here (mainly Dan and me 😉 ) agree that the current restriction can be relaxed - the rest is bikeshedding about how much of a problem this whole thing is. Would you say this is correct? If yes - I would probably get into adjusting transforms soon and we could bikeshed during code review.

[Andarist]

@gaearon It seems that the use case you have mentioned has not been considered at all - the implementation actually deliberately forces the user to choose the runtime. There is even a special pragma for that:

babel/packages/babel-helper-builder-react-jsx-experimental/src/index.js

Line 16 in b4ae7b7

const JSX_RUNTIME_ANNOTATION_REGEX = /\*?\s*@jsxRuntime\s+([^\s]+)/;

In addition to that, it seems that the new implementation (the one in @babel/helper-builder-react-jsx-experimental) changed the "rules" around classic runtime:

babel/packages/babel-helper-builder-react-jsx-experimental/src/index.js

Lines 256 to 259 in b4ae7b7

	throw new Error(
	"transform-react-jsx: pragma has been set but " +
	"pragmaFrag has not been set",
	);

There was no such restriction in the past in the old implementation - one could just use @jsx without @jsxFrag.

Both of those restrictions are problematic from Emotion's PoV and I have started to get reports about this being problematic since CRA 4 has been already released. What is your opinion about this and about the proposed solution?

[gaearon]

And for newcomers, this doesn't really sound like anything different. From what I understand the CRA has been created both as a newcomer-friendly 0-config tool and a reusable boilerplate for others - I would argue that the current state of things just isn't that newcomer-friendly.

I think my argument here is that "copy and paste this JSX comment into every file" is already not beginner-friendly, and a clear indication that you're breaking out of the default pattern. I'm not sure that beginners overwhelmingly choose Emotion when this manual workflow is involved. So I would argue that if they can copy and paste something into every file, they can also run a Replace-All codemod when they upgrade.

I have started to get reports about this being problematic since CRA 4 has been already released.

Can you please file an issue in the CRA repo to alert the maintainers? Ideally this should be added to "breaking changes" — that people need to wait for Emotion support or opt out of the new JSX transform (I believe there is an .env option for this now).

[gaearon]

dependency upgrades are always "risky" (semver being human-driven and all). So I believe that if we could avoid this then it would be a preferable situation because upgrading things gradually rather than multiple things at once is usually a slower but safer approach

I would go further and say it would be best for Emotion users to turn off the new transform after they upgrade CRA instead of rushing a solution on Babel level which is a behavior we'll have to live with forever.

[Andarist]

I certainly don't want to rush things on Babel - I genuinely believe that the proposed solution is slightly better than what got shipped. It's also not a rush idea based on my current problems - I have proposed pretty much the same back in March and it got positive feedback from @nicolo-ribaudo back then. It just has slipped in the flood of other comments etc - it was a pretty big PR after all.

How this currently works in Babel is also not consistent with what TS has implemented and I believe that matching the behavior regarding this between Babel and TS is a good goal, for the good of the whole community. However, TS has only shipped this in their beta versions so far - so one could argue that changes should be implemented there instead of here.

[gaearon]

How this currently works in Babel is also not consistent with what TS has implemented and I believe that matching the behavior regarding this between Babel and TS is a good goal, for the good of the whole community.

Yes but that's a separate issue. (We should report any discrepancies.) I think you filed one, was there something else too?

[Andarist]

Ive fixed the createElement not being autoimported from jsxImportSource module. So that was a big one and its already the same for Babel and TS.

The other discrepancy is that they do not support @jsxRuntime pragma at all (from what ive grepped in their code) and that regardless of the global config for classic/automatic runtime (its a jsx option in the tsconfig.json) it is enough to reconfigure that global runtime with the other one by just using @jsx/@jsxImportSource (its acting like a local - per file - override).

I have not reported this in their repo because I think their approach is better and I propose to match Babel’s behavior to theirs in that aspect in this issue here

[nicolo-ribaudo]

I'm trying to make sense of the different use cases posted, please correct me if I'm wrong:

• @gaearon proposes to make it easy to use both runtimes in the same file at different times, depending on the environment.

  /* @jsxImportSource jsx-library */
  /* @jsx h */
  import { h } from "jsx-library";
  <div />;

  // babel config
  export default api => ({
    plugins: [
      ["@babel/transform-react-jsx", {
        runtime: api.env() === "classic-runtime" ? "classic" : "automatic"
      }]
    ]
  });

  This would, depending on the env, produce one of these outputs:

  import { h } from "jsx-library";
  import { jsx as _jsx } from "jsx-library/jsx-runtime";
  _jsx("div");
  
  // or
  
  import { h } from "jsx-library";
  h("div");

• @Andarist proposes to make it possible using both the runtimes in different files at the same time, depending on what runtime library the file uses.

  // a.jsx
  /* @jsxImportSource react */
  <div />

  // b.jsx
  /* @jsx jsx */
  import { jsx } from "@emotion/core";
  <div />

  Which, regardless of the runtime set in the Babel config, is compiled to

  // a.jsx
  import { jsx as _jsx } from "react/jsx-runtime";
  _jsx("div");

  // b.jsx
  import { jsx } from "@emotion/core";
  jsx("div");

---

The problem we are discussing about is only during the transition period, and it's not clear why someone that successfully migrated their codebase to the "automatic" runtime would want to also run it in "classic" mode.

• If it is a library that hasn't control on the React version used by their user they should stick with the most compatible option, i.e. "classic".
• If it is an application that has control on which React version to use, they'll only start migrating to "automatic" after upgrading React to a compatible version.

I think that it's more common to have two different jsx factories: for example, I might start writing an app in pure React and then start using Emotion for some new components. I can understand that someone might want to migrate to the new runtime for React, and being able to use the @jsx pragma in the files where they are using Emotion makes it possible to migrate gradually. This would be similar to how we handle configuration files: options set nearer to the compiled file (for example, in the same directory) override options set further away (e.g. in the repo root).

However, there is also a third scenario to think about: someone wants to completely migrate to the automatic runtime but forgets a @jsx somewhere in their code.

---

The current behavior protects from the third scenario: if you opt-in into "runtime": "automatic" it prevents you from accidentally leaving the classic runtime around.

So my question is: does it actually harm using the two runtimes at the same time (e.g. maybe a bundle size increase?). If it doesn't, then based on my experience (i.e. using a jsx library only in some files) I think that optimizing for @Andarist's use case can be beneficial to more users.

[gaearon]

The concern is that this just kicks the can down the road. If people think that the transform has switched (but it has not), they will not get the runtime that is able to provide the necessary deprecation warnings for the long term (explained in the original thread). So they would lag behind everyone else who is fixing their key spread issues.

[Vadorequest]

I tried to upgrade Next.js from 9 to 10 today, alongside React 16.3 to 17.0.1, and I ran into this.

The underlying issue is related to Babel <> Emotion, as mentioned above.
There seems to be a workaround for theme-ui and CRA 4 as mentioned there:
system-ui/theme-ui#1160 (comment)

But this doesn't work with Next.js, the babel compilation fails whether I use:

• DISABLE_NEW_JSX_TRANSFOR=true in .env
• DISABLE_NEW_JSX_TRANSFORM=true in the package.json script
• /** @jsxRuntime classic */ on top of all files using /** @jsx jsx */

None of those solutions work at this time, I'm working on UnlyEd/next-right-now#189

I'm not sure if it's related to the Next.js framework, but my understanding after reading a few issues on the subject is that the above solutions should work.

I've opened an issue on the Next.js repository. vercel/next.js#18461

[Andarist]

@gaearon proposes to make it easy to use both runtimes in the same file at different times, depending on the environment.

This is IMHO quite a convoluted use case. I understand it and I wouldn't be super against it but I doubt that many users would actually use this - it's also not supported right now by Babel, so my proposal would not change anything regarding that but I guess it could prevent this being implemented further down the road without introducing breaking changes so it might be a good time to discuss this right now.

@Andarist proposes to make it possible using both the runtimes in different files at the same time, depending on what runtime library the file uses.

Yes. This is currently supported with the help of the extra @jsxRuntime pragma but I wouldn't call this super user-friendly (a lot more to type). But given this is required to use the classic runtime when automatic one is configured via config (and most likely vice-versa) this requirement made the whole thing breaking change (an unnecessary one in my opinion) for consumers of higher-level tooling.

This is also not how this got implemented in TS and given that TS 4.1 is supposed to be released within a week I would hope that we could act here quickly to align both implementations for the good of the ecosystem.

Note: I would have to test this to be sure but when I've been yesterday debugging some runtime-related issue in TS I came to the conclusion that if both pragmas are specified in a file:

/** @jsx h */
/** @jsxImportSource @emotion/react */

then the classic runtime is being used.

This would be similar to how we handle configuration files: options set nearer to the compiled file (for example, in the same directory) override options set further away (e.g. in the repo root).

Exactly my line of thinking here.

However, there is also a third scenario to think about: someone wants to completely migrate to the automatic runtime but forgets a @jsx somewhere in their code.

Well, this won't introduce a lot of negatives as long as the underlying library supports it. With time we could add a deprecation warning to Emotion's jsx factory. I also don't think this is necessarily different from any other migration related to any other library. This one might even be easier to do because grepping this in the codebase is easier with this one than for regular functions/methods

So my question is: does it actually harm using the two runtimes at the same time (e.g. maybe a bundle size increase?). If it doesn't, then based on my experience (i.e. using a jsx library only in some files) I think that optimizing for @Andarist's use case can be beneficial to more users.

It probably hurts bundlesize a little bit - but personally, I wouldn't be overly cautious about this one. Smooth migration is IMHO more important than some extra bytes (and I'm a bytes-concerned guy 😅 ).

The concern is that this just kicks the can down the road. If people think that the transform has switched (but it has not), they will not get the runtime that is able to provide the necessary deprecation warnings for the long term (explained in the original thread). So they would lag behind everyone else who is fixing their key spread issues.

@gaearon How they can think that if they are using @jsx pragma? I understand that one can forget to remove this somewhere but when working on the codebase one should remove this over time - like I've mentioned: this feels to me like any other migration that just has to be done over time.

I'm not sure if it's related to the Next.js framework, but my understanding after reading a few issues on the subject is that the above solutions should work.

Yes, one of those should probably work. If you share a runnable repro case of your problem I might take a look at it. I've just skimmed through the issues you have mentioned but couldn't find such quickly - if it's there then please just link me to it.

---

While I still believe that my proposal has merit and I would love to see it being implemented (which I'm offering to do), the more pressing issue for me at the time is that those semantics should be aligned between Babel and TS and they clearly are not right now. So changes in one of the tools are required (or in both?) - I could try to work on both (although I'm feeling much more comfortable in making changes to Babel than to TS) but I'd really need to know what is the end game here.

@gaearon could you discuss this with the React team? Or maybe you have already done that and the team's opinion of what has been shipped didn't change? What should be done with TS implementation? I'm happy to discuss this over a call or something to just establish what should be the final semantics of this without dragging this issue further over time.

@nicolo-ribaudo regarding all presented arguments from both sides - what's your stance on this? are you inclined to support one of the proposed approaches over another? or would you feel better if the React team was to decide about this?

[nicolo-ribaudo]

Re-reading the whole thread, I prefer the semantics that @Andarist is proposing.

The configuration defined inline in a file should always take precedence over what is defined in the configuration files, and an explicit @jsx/@jsxImportSource pragma is a strong signal of the runtime that the user needs in that file.

In cases where there is a real conflict (i.e. two incompatible options in the config, or two incompatible pragmas in the source file) we should at least warn.

However, I would accept a veto from the React team (but only if the TS implementation is changed to match the same semantics).

cc @weswigham In case you aren't aware of this discussion: currently the TS and Babel semantics are different, and we should try to align on the same behavior.

[weswigham]

@Andarist is currently working on adjusting the TS semantics a smidge to match this, and I'm open to the change, since it's pretty reasonable from a compatibility perspective.

[Andarist]

@weswigham this is not quite accurate. One bug I've fixed has indeed fixed a compatibility issue with Babel but the current PR that I'm working on is focused on JSX namespace resolution and this is TS-specific.

The issue here is about:

• whether the possibility to use a particular pragma should be restricted by the config option (or if an additional @jsxRuntime pragma should be required for the other pragma to work)
• what should happen if one uses both @jsx and @jsxImportSource pragmas at the same time (in Babel the behavior is currently different based on the config option because it switches the implementation entirely) and

[JLHwung]

I think we all agree expected result on situation 1: @jsxImportSource should be respected in both runtime.

I agree with @Andarist on situation 2. The pragma, by definition, should override config. I acknowledge that React teams worry that deprecation warnings are not shown for components run by classic runtime. We could introduce an option allowJsxClassicPragmaInAutomatic: boolean (defaulted to false) since it is not recommended for using classical runtime alongside the automatic one. By specifying that users opt in to such behaviours which may causes larger bundle size and loss of some deprecation warnings.

As for automatic runtime pragma inside classical runtime, I think it should be encouraged since it can be the case when users start migrating to the new runtime. In other words, allowJsxClassicPragmaInAutomatic only works for runtime: "automatic".

On situation 3 where conflict pragma is detected, I agree that we should at least warn.

[nyngwang]

I would like to know what was the ending of this thread then. Everything frozen in the end of 2020...?