-
-
Notifications
You must be signed in to change notification settings - Fork 189
fix(react): respect tsconfig jsxImportSource by default #726
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
Changes from 2 commits
a6aac50
904f10a
79eefce
7e2d7b0
24f6386
d6ad519
d6feee0
e53f1f1
16e4ecd
ea00b55
6c65e50
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import { expect, test } from 'vitest' | ||
import { page } from '~utils' | ||
|
||
test('respect tsconfig jsxImportSource', async () => { | ||
await expect | ||
.poll(() => | ||
page.getByTestId('emotion').evaluate((el) => getComputedStyle(el).color), | ||
) | ||
.toBe('rgb(255, 0, 0)') | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
<div id="app"></div> | ||
<script type="module" src="./src/main.tsx"></script> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
{ | ||
"name": "@vitejs/test-react-tsconfig", | ||
"private": true, | ||
"type": "module", | ||
"scripts": { | ||
"dev": "vite", | ||
"build": "vite build", | ||
"debug": "node --inspect-brk vite", | ||
"preview": "vite preview" | ||
}, | ||
"dependencies": { | ||
"@emotion/react": "^11.14.0", | ||
"react": "^19.1.1", | ||
"react-dom": "^19.1.1" | ||
}, | ||
"devDependencies": { | ||
"@types/react": "^19.1.9", | ||
"@types/react-dom": "^19.1.7", | ||
"@vitejs/plugin-react": "workspace:*" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
export default function App() { | ||
return ( | ||
<div data-testid="emotion" css={{ color: 'rgb(255, 0, 0)' }}> | ||
This should be red | ||
</div> | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import ReactDOM from 'react-dom/client' | ||
import App from './App.jsx' | ||
|
||
ReactDOM.createRoot(document.getElementById('app')!).render(<App />) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
{ | ||
"include": ["src"], | ||
"compilerOptions": { | ||
"target": "ES2020", | ||
"useDefineForClassFields": true, | ||
"lib": ["ES2020", "DOM", "DOM.Iterable"], | ||
"types": ["vite/client"], | ||
"module": "ESNext", | ||
"skipLibCheck": true, | ||
|
||
/* Bundler mode */ | ||
"moduleResolution": "bundler", | ||
"allowImportingTsExtensions": true, | ||
"resolveJsonModule": true, | ||
"isolatedModules": true, | ||
"noEmit": true, | ||
"jsx": "react-jsx", | ||
"jsxImportSource": "@emotion/react", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can update the playground/emotion to comment the manual jsxImportSource (saying we expect to be infered from tsconfig) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea. I should reuse that but I'm not sure how to get esbuild to pickup tsconfig for jsx. I'll update the example to tsx in a later PR and remove this one. |
||
|
||
/* Linting */ | ||
"strict": true, | ||
"noUnusedLocals": true, | ||
"noUnusedParameters": true, | ||
"noFallthroughCasesInSwitch": true | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import react from '@vitejs/plugin-react' | ||
import { defineConfig } from 'vite' | ||
|
||
export default defineConfig({ | ||
plugins: [react()], | ||
}) |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
I needed to remove
runtime
too since rolldown-vite merge oxc option with tsconfig only whenruntime === undefined
. Is this intended or allow alsoruntime === 'automatic'
? @sapphi-red https://github.com/vitejs/rolldown-vite/blob/f88cb05e755457fd9ea99174a017c49964510c3f/packages/vite/src/node/plugins/oxc.ts#L65-L68There 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.
With the vite-rolldown code as is, this would mean that users having
jsx === 'preserve'
in tsconfig will now have broken apps I think.Is this also the same logic for the esbuild?
Uh oh!
There was an error while loading. Please reload this page.
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.
Ah, I didn't know react plugin is ensuring
automatic
so that tsconfig's jsx preserve will be ignored. Probably rolldown-vite oxc option merging needs to be tweaked. I'll also add that test case too then.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.