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.

Sign up for GitHub

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

refactor(remix-dev/vite): remove redundant jsx and typescript babel plugins #7998

Merged
merged 5 commits into from
Nov 30, 2023
Merged

refactor(remix-dev/vite): remove redundant jsx and typescript babel plugins #7998

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
80307b1
refactor(remix-dev/vite): remove "jsx" and "typescript" babel plugins
hi-ogawa Nov 14, 2023
325f3ae
refactor: remove "remix-react-refresh-babel" plugin from child compiler
hi-ogawa Nov 14, 2023
6a1b0c1
Merge branch 'dev' into refactor-vite-remove-remix-react-refresh-babe…
hi-ogawa Nov 29, 2023
3d3b476
chore: partial revert
hi-ogawa Nov 29, 2023
f4f1b06
Merge branch 'dev' into refactor-vite-remove-remix-react-refresh-babe…
markdalgleish Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/remix-dev/vite/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1049,7 +1049,7 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
},
{
name: "remix-react-refresh-babel",
enforce: "post",
enforce: "post", // jsx and typescript (in ts, jsx, tsx files) are already transpiled by vite
async transform(code, id, options) {
if (viteCommand !== "serve") return;
if (id.includes("/node_modules/")) return;
Expand All @@ -1069,7 +1069,6 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
parserOpts: {
sourceType: "module",
allowAwaitOutsideFunction: true,
plugins: ["jsx", "typescript"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we still want this change. Can we reopen this PR and scope it down to just this one liner?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for re-opening! I updated a PR to include only this change.

Btw, recently macos/webkit CI is often failing with vite-dotenv-test.ts but I'm not sure if it's related to my PR. Is this something I should worry about?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hi-ogawa I'm looking into that test failure now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR got the test passing: #8177. I've just merged dev into your PR.

},
plugins: [[require("react-refresh/babel"), { skipEnvCheck: true }]],
sourceMaps: true,
Expand Down
Loading