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

Let jsxImportSource be configurable through tsconfig.json #9847

Open
1 task done
Andarist opened this issue Oct 21, 2020 · 33 comments
Open
1 task done

Let jsxImportSource be configurable through tsconfig.json #9847

Andarist opened this issue Oct 21, 2020 · 33 comments

Comments

@Andarist
Copy link
Contributor

Is your proposal related to a problem?

Right now it's only suggested for tsconfig.json to set "jsx": "react" but it's not enforced.

TS 4.1 will bring new JSX-related option (jsxImportSource) and new possible values for the jsx option (react-jsx & react-jsxdev) - both of those changes are related to the new JSX runtimes.

TS also has an ability to change what exact JSX namespace is used for type-checking based on the used jsx factory as per the documentation. This probably means that there is a problem for some possible configuration today:
TS can do its typechecking thinking that another factory is used (when one would configure jsxFactory) but in fact the emitted code uses the default factory of React.createElement because the transpilation is done by Babel that is not aware of TS configuration.

I'm not really interested in fixing this problem - I haven't found any issue about this so the audience for this is probably very little and it's just a possible problem that I have thought of and not something that affects me.

However, I would really love to be able to configure jsxImportSource globally and while you don't allow for Babel customization (which is understandable tradeoff here) you allow for some tsconfig.json configuration. It (tsconfig.json) seems to be a good match for inferring this particular configuration for a vast number of consumers (as TS is used by a lot of people).

Motivation

I'm a maintainer of Emotion which is a popular CSS-in-JS solution for React (3.4M downloads/week) which comes with the CSS prop API that is based on @jsx pragma (although not all of our consumers use the CSS prop API as we expose the popular styled API as well).

Right now the @jsx pragma in CRA has to be used all over the place. We personally don't find it to be that problematic but we get complaints about this regularly from the community. Having a way to configure this (new transforms, not the old pragma) globally in CRA would probably result in a huge DX improvement for us.

Describe the solution you'd like

Provide custom importSource to @babel/preset-react, based on the jsxImportSource from tsconfig.json

Describe alternatives you've considered

If this is something that you don't want to support then I strongly believe that you should disallow configuring jsxImportSource in the tsconfig.json altogether because it causes a configuration mismatch between TS and Babel and this might be a footgun for people.


  • Implementation intent
@seanbruce
Copy link

I created a new project using CRA4 with typescript template today.
I have a difficult time trying to use emotion. as I know with the release version 10.1.0 of emotion, I should use /** @jsxImportSource @emotion/core /` instead of /* @jsx jsx */, so I try to use like this

/** @jsxImportSource @emotion/core */
import { jsx } from '@emotion/core';

or just

/** @jsxImportSource @emotion/core */

but I got error anyway saying "'React' refers to a UMD global, but the current file is a module. Consider adding an import instead.". I guess maybe it's about CRA typescript template.

I'm not familiar with neither CRA nor new JSX runtime.

@Andarist
Copy link
Contributor Author

Andarist commented Nov 1, 2020

Could u share a repository with this issue reproduced? I could take a look at this.

@seanbruce
Copy link

@Andarist Thanks! here is the repository https://github.com/seanbruce/butler-of-ellen.git

@Andarist
Copy link
Contributor Author

Andarist commented Nov 1, 2020

The current TS version is not aware of the new @jsxImportSource and thus it doesn't know that there are already implicit imports (inserted automatically) based on that @jsxImportSource pragma. This requires TS 4.1 which is currently at the beta stage and I believe that it is supposed to be shipped as stable this month.

I've managed to trick the compiler to accept this by changing this:
https://github.com/seanbruce/butler-of-ellen/blob/28754637234e145547f16232fe5eef111457a651/tsconfig.json#L18
to "preserve"

And by adding import '@emotion/core' to this file:
https://github.com/seanbruce/butler-of-ellen/blob/28754637234e145547f16232fe5eef111457a651/src/react-app-env.d.ts

@seanbruce
Copy link

following this trick. I change compilerOptions.jsx to "preserve" and add change react-app-env.d.ts like this

import '@emotion/core';
/// <reference types="react-scripts" />

then I run yarn start I got this error says TypeError: Cannot assign to read only property 'jsx' of object '#<Object>'

@Andarist
Copy link
Contributor Author

Andarist commented Nov 2, 2020

Oh, right 😢 I've focused on fixing TS problems without running yarn start and CRA enforces certain values to be what they want them to be. Those are enforced values:

jsx: {
parsedValue:
hasJsxRuntime && semver.gte(ts.version, '4.1.0-beta')
? ts.JsxEmit.ReactJSX
: ts.JsxEmit.React,
value:
hasJsxRuntime && semver.gte(ts.version, '4.1.0-beta')
? 'react-jsx'
: 'react',
reason: 'to support the new JSX transform in React 17',
},

I'll give this more thought but I don't have an idea how this can be approached right now without waiting for TS 4.1

@seanbruce
Copy link

@Andarist I think I just wait for TS4.1 released to solve this issue once and for all. for now, I use this not perfect solution until TS4.1. Thank you so much for your time.😄

@MatthieuCoelho
Copy link

MatthieuCoelho commented Dec 3, 2020

With version Typescript 4.1 I tried

"jsxImportSource": "@emotion/react",
"jsx": "react-jsx"

But the css props from emotion is not working, I have to put "/** @jsxImportSource @emotion/react */" on top on the file.

Is the property jsxImportSource ignored ?

@Andarist
Copy link
Contributor Author

Andarist commented Dec 3, 2020

And this issue is exactly about this problem. I'm proposing that the CRA's Babel preset could be "configured" with the importSource based on the jsxImportSource of the tsconfig.json. I believe that wouldn't compromise the 0config approach taken by the CRA.

@stephenh
Copy link

@iansu fyi this feature (being able to pass importSource to babel) would be a big usability win for CRA's CSS-in-JS users (i.e. emotion/et al) by being able to remove the /** jsxImportSource ... */ lines that currently have to be copy/pasted into every file in a project.

It looks like this PR added an importSource with a process.env.JSX_IMPORT_SOURCE flag:

https://github.com/facebook/create-react-app/pull/10138/files#diff-7cd8a2246cedc973ca54463e12073aaff89e13fda0f3d8b61742ec8e2b64b708R100

But @Andarist is suggesting adding a smarter if-isTypeScriptEnabled-then-parse-tsconfig-for-its-jsxImportSource setting, right around this section of code from your "New JSX Transform Opt Out" PR:

https://github.com/facebook/create-react-app/blame/master/packages/babel-preset-react-app/create.js#L99

If I wrote a PR that changed that ~section of code to conditionally parse tsconfig.json and pass along importSource to babel, would that likely get accepted? Do you have any preferences/suggestions for a better approach?

Thanks!

(...I'm a little worried about "just fs.load tsconfig.json; while really simple, it wouldn't support tsconfigs that extend other tsconfigs ... would have to research the idiomatic way of loading tsconfigs...although maybe that could be a V2.)

@Andarist
Copy link
Contributor Author

TS exposes the API to read the config file so that part shouldn't be particularly a problem:

const { config: readTsConfig, error } = ts.readConfigFile(
paths.appTsConfig,
ts.sys.readFile
);

@onpaws
Copy link

onpaws commented Feb 1, 2021

@stephenh Curious if you had the chance to start the PR you mentioned?
I just created a new CRA4 app + Emotion 11 and ran into this - completely agree it will be awesome for CRA users if they don't have to manually prepend
/** @jsxImportSource @emotion/react */
on a per-file basis.

@onpaws
Copy link

onpaws commented Feb 1, 2021

Is the property jsxImportSource ignored ?

@MatthieuCoelho Yes, it seems that way. On CRAv4.0.1 + Emotion v11.1.4 I was able to repro what you're describing - this property seems to not get picked up by react-scripts which makes sense since it only arrived with TS v4.1+.

Is anyone aware of existing PRs that start to make CRA aware of TypeScript 4.1?

@stephenh
Copy link

stephenh commented Feb 1, 2021

@onpaws no, didn't bother starting a PR w/o an ack from the maintainers. Plus our existing codebase already has the per-file snippet so this is a nice to have, but not blocking us.

@onpaws
Copy link

onpaws commented Feb 1, 2021

Got it, thanks for the prompt reply.

Plus our existing codebase already has the per-file snippet so this is a nice to have, but not blocking us.

Lucky you, wish I could say the same! 🙈

If I get some time in the coming weeks I'll try to take a look at 1, 2 for other potential ways to workaround it.

@ben-rogerson
Copy link

If you're happy using craco/rewired so you can edit the babel config then you should be able to get around the per-file pragma definition with the trick I've used for twin.

@hermansje
Copy link

I opened a PR that uses the jsxImportSource compiler option from tsconfig.json or removes it (and explains why) when Babel and TypeScript configuration would be inconsistent: #10583

@pedrosimao
Copy link

Any progress on this one?

@donaldkg
Copy link

This issue is confusing and some documents say this typescript config works but actually it is not. Do you have any updates on this issue?

@55Cancri
Copy link

55Cancri commented May 17, 2022

I'm currently running a vite application and the emotion docs (https://emotion.sh/docs/typescript) suggest updating the tsconfig compiler options with:

{
    "jsx": "react-jsx",
    "jsxImportSource": "@emotion/react",
}

but this does not work- I still need to add /** @jsxImportSource @emotion/react */ above each file. What am I missing?

@gimurpe
Copy link

gimurpe commented Nov 17, 2022

Any news on this ?

@guopingxiao
Copy link

guopingxiao commented Dec 1, 2022

need to add /** @jsxImportSource @emotion/react */ above each file is very hard for devlopers

@ALFmachine
Copy link

👍

@baba43
Copy link

baba43 commented Feb 20, 2023

Oh man, this problem is really annoying for the developer experience. In our project, the css prop worked for a long time, until some update. Unfortunately, the bug is noticed only in the application and not in the development process.

I don't want to complain, because I am very thankful for all the time invested in open source tools here. However, this problem is so prominent in Google searches that it really should be addressed.

@claudiamatosa
Copy link

claudiamatosa commented Mar 1, 2023

In case this helps anyone: if your configuration is ejected and you have access to the Webpack config file, you can use the ModifySourcePlugin to prepend the jsxImportSource comment to every file:

new ModifySourcePlugin({
  rules: [
    {
      test: /\.tsx$/i,
      operations: [
        new ConcatOperation(
          'start',
          '/** @jsxImportSource @emotion/react */\n\n'
        )
      ]
    }
  ]
}),

I added it at the start of the plugins list, which seemed to do the trick. It's not ideal, but I couldn't bear the thought of adding this at the top of every file. 😅

@Gittified
Copy link

Is there any update on this issue? It's been a problem for a while, and I can not figure out how to use the CSS prop without manually adding the JSX Pragma in every file. It is extremely annoying to add it to every file and if anyone has a solution that does not involve prepending the pragma using a Webpack plugin it would be much appreciated! I am also using CRACO, but no luck getting it to work.

@finnhvman
Copy link

Would be awesome to have an inline CSS solution out of the box. Svelte/SvelteKit does this the best way to my knowledge: no extra steps needed, you can just write standard CSS within your component and they are scoped to it by default, even the classes.

@TamirCode
Copy link

I do not want to add /** @jsxImportSource @emotion/react */ at the top of every file in my CRA project. Not only because its extra work, and we as developers should find solutions for repetitive tasks, but also because it looks hideous and hacky.

Does anyone have a step by step work-around this issue?

@55Cancri
Copy link

I do not want to add /** @jsxImportSource @emotion/react */ at the top of every file in my CRA project. Not only because its extra work, and we as developers should find solutions for repetitive tasks, but also because it looks hideous and hacky.

Does anyone have a step by step work-around this issue?

Yes, the solution is simple: switch to vite yesterday and start using stitches instead of emotion. All your problems will be solved overnight.

@TamirCode
Copy link

I do not want to add /** @jsxImportSource @emotion/react */ at the top of every file in my CRA project. Not only because its extra work, and we as developers should find solutions for repetitive tasks, but also because it looks hideous and hacky.
Does anyone have a step by step work-around this issue?

Yes, the solution is simple: switch to vite yesterday and start using stitches instead of emotion. All your problems will be solved overnight.

I like to use inline-styles like this, unfortunately stitches doesnt work like this.

			<div
				css={{
					backgroundColor: 'pink',
					height: "500px",
					'&:hover': {
						backgroundColor: 'green'
					}
				}}
			>
			</div>

@TamirCode
Copy link

I do not want to add /** @jsxImportSource @emotion/react */ at the top of every file in my CRA project. Not only because its extra work, and we as developers should find solutions for repetitive tasks, but also because it looks hideous and hacky.

Does anyone have a step by step work-around this issue?

ive answered my own question here
https://stackoverflow.com/a/76381748/12671440

@55Cancri
Copy link

55Cancri commented Jun 1, 2023

I do not want to add /** @jsxImportSource @emotion/react */ at the top of every file in my CRA project. Not only because its extra work, and we as developers should find solutions for repetitive tasks, but also because it looks hideous and hacky.
Does anyone have a step by step work-around this issue?

Yes, the solution is simple: switch to vite yesterday and start using stitches instead of emotion. All your problems will be solved overnight.

I like to use inline-styles like this, unfortunately stitches doesnt work like this.

			<div
				css={{
					backgroundColor: 'pink',
					height: "500px",
					'&:hover': {
						backgroundColor: 'green'
					}
				}}
			>
			</div>

Yes I used to write this way too. Now with stitches I can do:

return  <Button
              variant="naked"
              cols="mx mx" // mx expands to max-content
              colG={12}
              alignI // true defaults to center
              h={500} // alias for height
              w="fit-content"
              px={16}
              py={8}
              mx={8} // margin left and right
              br={24}
              bg={theme.colors.pink9}
              initial={{ opacity: 0 }} // even exposes framer-motion props
              animate={{ opacity: 1 }}
              exit={{ opacity: 0 }}
              disabled={isLoading}
              _disabled={{
                 bg: theme.colors.gray5
              }}
              _before={{
                content: "",
                position: "absolute",
                inset: 0
             }}
             _hover={{
                bg: theme.colors.green9
             }}
             _active={{
               scale: 0.9
             }}
            _notFocusOutline: {{ outline: "none" }} // even really complex css selector becomes a single prop. This expands to `&:not(:focus-visible)`
            css={{
               svg: {
                  w: "100%"
               }
            }}
            >
              <Text color={theme.colors.red8}>Delete</Text>
            </Button

The above is more of a kitchen sink example, but with stitches I can do your same stuff but use my own aliases for css properties AND values. cols gets translated to gridTemplateColumns and mx expands to max-content.

w, h, px, and bg become width, height, padding-left and padding-right, and background-color underneath.

_disabled, _active, and _hover become &:disabled, &:active, and &:hover, respectively, but I could have made them anything I wanted.

No more dealing with the limitations of CRA and emotion. I get to make my own CSS-in-JS shorthands. It's skyrocketed my UI development. Now I only use css prop to target child elements (and css prop still works with stitches)

I even added an onLongPress handler and copy variant to my button, and I have cool things for many of my components now. I have a truncate prop for my <Text> component (just a stitches created p tag underneath) that will apply at least 3 css classes to give me the ellipsis when text is too long. vite + stitches + framer motion + Radix colors has made all the difference.

There is a little setup required but its so insanely worth it.

@johannes-xerxes-sz
Copy link

Tried doing this and it works

// tsconfig.json
{
  "compilerOptions": {
    // ...
    "jsxImportSource": "@emotion/react",
    "jsx": "react-jsx"
  }
}
// vite.config.ts
export default defineConfig({
  plugins: [react({ jsxImportSource: '@emotion/react' })],
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests