feat(plugin-react): change how babel.include/exclude options behave

[aleclarson]

If someone wants to write tests for this, you're more than welcome! 😄

Description

By default, Babel plugins passed to plugin-react are only applied to modules in the project root (as well as virtual modules), except when they live in a node_modules folder. To override this behavior, you can now define babel.include to ensure a module is transformed by those particular plugins (note that babel.config.js can be used to apply plugins/presets to your entire bundle, but you need to pass configFile: true to plugin-react first). Similarly, you can define babel.exclude to stop a module from being transformed by the Babel plugins passed to plugin-react (as well as .babelrc plugins).

reactPlugin({
  babel: {
    babelrc: true,
    plugins: [...],
    // Apply plugins above (and plugins in .babelrc) to all node_modules
    include: ['**/node_modules/**'],
    // Skip applying plugins to "src/foo" folder
    exclude: [/src\/foo/],
  }
})

There's also a breaking change in this PR: The include/exclude options have been moved into the fastRefresh option, so as to avoid confusing users into thinking they affect Babel transformation.

Additional context

Closes vitejs/vite-plugin-react#16

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[NMinhNguyen]

If I understand this correctly, this would still place the burden on the consumer to correctly specify a regex/glob pattern to include/exclude files? Is it possible for monorepo setups (at least the most common ones) to work out of the box? Perhaps by excluding node_modules by default but including everything else?

[aleclarson]

@NMinhNguyen Good point! We could look for monorepo config files (eg: pnpm-workspace.yaml) and include their local directory globs by default. How does that sound?

[NMinhNguyen]

@NMinhNguyen Good point! We could look for monorepo config files (eg: pnpm-workspace.yaml) and include their local directory globs by default. How does that sound?

Is checking for node_modules not sufficient/brittle?

[aleclarson]

Is checking for node_modules not sufficient/brittle?

The majority use case is interested in applying Babel plugins to project files only. I think it makes sense for monorepo users to explicitly opt-in specific local packages to be parsed/transformed, for performance reasons. So, on second thought, I think checking for workspace configs by default is unnecessary, but there could be an option for that.

For a monorepo user, it's as easy as this:

reactPlugin({
  babel: {
    include: ['..']
  }
})

Or you can put babel.config.js in your monorepo root, if the Babel config does not rely on Vite context.

[NMinhNguyen]

The majority use case is interested in applying Babel plugins to project files only.

Isn't "project files only" roughly equivalent to !.includes('/node_modules/')? But also CRA applies Babel to node_modules too to allow packages to publish the latest ESNext code to npm and then be processed according to browserslist config in your app.

I think it makes sense for monorepo users to explicitly opt-in specific local packages to be parsed/transformed

I'm essentially using this internal packages approach from https://turborepo.com/posts/you-might-not-need-typescript-project-references where I'd like to author the majority of packages in TSX + Emotion and for them to be automatically processed in my Vite app.

[aleclarson]

Isn't "project files only" roughly equivalent to !.includes('/node_modules/')?

No it's more like startsWith(viteConfig.root + '/') && !includes('/node_modules/')

A local package being referenced by the project doesn't imply that the project's Babel plugins should be applied to it. Better not to assume that, for best performance.

CRA applies Babel to node_modules too to allow packages to publish the latest ESNext code to npm and then be processed according to browserslist config in your app.

Vite uses Esbuild to compile node_modules for production builds, so support for esnext syntax in node_modules is enabled by default.

As for browserslist, it's recommended to generate two production builds (modern and legacy) if you want to support older browsers. Use @vitejs/plugin-legacy to accomplish that. It supports browserslist configs by default.

I'd like to author the majority of packages in TSX + Emotion and for them to be automatically processed in my Vite app.

Have you tried using babel.config.js in your monorepo root? That should work for you.

You'll only need to add the Emotion babel plugin, since TSX is compiled by Esbuild by default (even for node_modules).

[NMinhNguyen]

A local package being referenced by the project doesn't imply that the project's Babel plugins should be applied to it. Better not to assume that, for best performance.

Ah I see what you mean, so you don't want to apply the project's Babel plugins to the rest of the monorepo. I see now.

Have you tried using babel.config.js in your monorepo root? That should work for you.

Yes, I have it, but it doesn't seem to be applied to packages outside of the project root because of the startsWith(viteConfig.root + '/') check.

[aleclarson]

it doesn't seem to be applied to packages outside of the project root because of the startsWith(viteConfig.root + '/') check.

You have to pass configFile: true to plugin-react. (Probably need to make that the default)

[NMinhNguyen]

You have to pass configFile: true to plugin-react. (Probably need to make that the default)

Ah that worked (I also set rootMode: 'upward')! But if the root babel.config.js includes things like @babel/preset-env, @babel/preset-react, @babel/preset-typescript, will that not be an issue? And what is it that makes it apply babel.config.js to everything? Is it the case that by default plugins is empty, but we still run the Babel Rollup plugin on every file?

[aleclarson]

But if the root babel.config.js includes things like @babel/preset-env, @babel/preset-react, @babel/preset-typescript, will that not be an issue?

Those plugins will be applied, so your build will be slower, but it shouldn't break anything.

You can set process.env.VITE = '1' in your vite.config.js file, then look for that in your babel.config.js file. If its value is truthy, you would avoid using those unnecessary plugins.

And what is it that makes it apply babel.config.js to everything?

That's a Babel convention, not Vite's doing.

Is it the case that by default plugins is empty, but we still run the Babel Rollup plugin on every file?

1. We use Babel directly, instead of using the @rollup/plugin-babel package.
2. There's a shouldSkip boolean further down that file which checks for empty plugins array.

vite/packages/plugin-react/src/index.ts

Lines 226 to 229 in 8fb854f

	const shouldSkip =
	!plugins.length &&
	!babelOptions.configFile &&
	!(isProjectFile && babelOptions.babelrc)

That said, because configFile: true is used, @vitejs/plugin-react will transform every single file, since it can't know which plugins (if any) are actually applied by the babel.config.js files that are found. I'm not sure if Babel is smart enough to avoid parsing a file if no plugins are active.

Btw, you'll probably want to use the exclude in your babel.config.js to avoid applying its plugins to node_modules.

[NMinhNguyen]

@aleclarson that was really insightful, thanks! I did notice major slowdowns with configFile: true so will see if exclude helps but I think you're right and it's better to not apply Babel to files outside the project (for perf).

[caghand]

Should this be "exclude" instead of "include"?

[aleclarson]

No but it does need a comment to clarify that :)

It's creating a function that returns true if a babel.exclude pattern is matched.

[caghand]

Ahh, got it, thanks!

[caghand]

Would any "exclude" settings be ignored in this case?

[aleclarson]

Good observation! With how this is currently written, one cannot exclude a subset of paths matched by include, which is certainly a valid use case.

Should be a simple fix, just add exclude: opts.babel?.exclude to the shouldProjectInclude definition.

[caghand]

Hi everyone,
Yes, the babel.config.js solution is pretty useful, but I think it's not the correct way to make plugin-react support monorepos.
I wouldn't want to run babel.config.js plugins on node_modules. OK, I can exclude that directory. But then, this exclude setting gets merged with the programmatic configuration in plugin-react, causing it to skip the JSX transforms that are done on some node_modules files (for the automatic runtime).
Or, at the other extreme, let's say that I don't want to change the Babel configuration, and I want to use plugin-react's out-of-the-box settings. Then, if a source code file is outside the project root, has the .js or .ts extension, and contains React.createElement(), then plugin-react will slap the babel-restore-jsx plugin onto it, causing the subsequent vite steps to fail. This forces me to change the extensions of these files to .jsx or .tsx. But I shouldn't need to do that. (And there is no way to know that without reading the source code.)
In my opinion, the correct way to support monorepos is to change the exact definition of isProjectFile... which is what this PR does. :)

[aleclarson]

But then, this exclude setting gets merged with the programmatic configuration in plugin-react

I don't think that's true. The exclude setting of babel.config.js is only ever used by Babel internals.

Then, if a source code file is outside the project root, has the .js or .ts extension, and contains React.createElement(), then plugin-react will slap the babel-restore-jsx plugin onto it, causing the subsequent vite steps to fail.

That shouldn't be happening. Do you have a repro?

[caghand]

Hi @aleclarson,

But then, this exclude setting gets merged with the programmatic configuration in plugin-react

I don't think that's true. The exclude setting of babel.config.js is only ever used by Babel internals.

I'm sorry, this specific problem happens when I use the ignore setting in babel.config.json (not the exclude setting).
In the configuration object you are passing to transformAsync(), you are including the babel.config.json reference as well as your programmatically-added plugins. Therefore, the ignore setting in babel.config.json gets applied to your programmatic configuration as well.
(If I use exclude instead of ignore, then it only gets applied to babel.config.json. But because the existence of babel.config.json causes the shouldSkip variable in plugin-react to become false, I won't be able to stop Babel from running on node_modules. Therefore, I must use ignore instead of exclude.)
This is a flaw in plugin-react's handling of babel.config.json, and is independent of whether I am using a monorepo. I mean, you don't need a monorepo to reproduce it. You just need to use a package that has a *.js file with React.createElement() in it.
For example, in an empty Vite project (with plugin-react), add @fluentui/react, as well as React 17. In vite.config.js, add "@fluentui/react" to optimizeDeps.exclude, so that you can observe the transformation of the individual ES modules. In the main JSX file of your project, add a simple <DefaultButton>, imported from @fluentui/react. At this time, take a look at the original version of DefaultButton.js under node_modules/@fluentui/react/lib/components/Button/DefaultButton/. You will see React.createElement() in it. Now, vite serve should run fine. plugin-react will transform DefaultButton.js using the Babel plugins babel-plugin-transform-react-createelement-to-jsx and @babel/plugin-transform-react-jsx-development. In your browser's DevTools, you can see the transformed version of DefaultButton.js. You will see that React.createElement() has been translated to _jsxDEV().
Now, create a babel.config.json file, and add "ignore": ["**/node_modules/**"] to it. In the plugin-react settings in vite.config.js, add the babel setting configFile: true. Now, if you run vite serve, you will see that DefaultButton.js has not been transformed. The React.createElement() statement is untouched.
So, by using babel.config.json, I have broken an existing feature. :)

Then, if a source code file is outside the project root, has the .js or .ts extension, and contains React.createElement(), then plugin-react will slap the babel-restore-jsx plugin onto it, causing the subsequent vite steps to fail.

That shouldn't be happening. Do you have a repro?

Sure, here it is: https://codesandbox.io/s/vite-react-monorepo-x2gldz
Please download this sandbox, install the NPM packages, and run vite serve from packages/app/.
The problem happens when using TypeScript.
You will see that Header.tsx works fine but Footer.ts fails, because its extension is .ts instead of .tsx, and it contains React.createElement(). plugin-react tries to activate babel-plugin-transform-react-createelement-to-jsx for it, but without a TypeScript parser. This would work fine for node_modules files, because they don't need additional parsers, but it fails for "project files", because they need additional parsers. Footer.ts is a "project file"; it shouldn't be treated like a node_modules file.

Thanks!

[caghand]

(If I use exclude instead of ignore, then it only gets applied to babel.config.json. But because the existence of babel.config.json causes the shouldSkip variable in plugin-react to become false, I won't be able to stop Babel from running on node_modules. Therefore, I must use ignore instead of exclude.)

Maybe a possible improvement is the following:
If plugins.length is 0, then, even if babel.config.json exists, you pass an exclude property to transformAsync(), containing the current file.
I think this would prevent Babel from running unnecessarily on files that would normally be skipped by plugin-react.
Then, babel.config.json would use test/include and exclude to manage its own domain, independently of the include/exclude logic of the programmatic configuration in plugin-react. This is in contrast to using only and ignore in babel.config.json, which impact the programmatic configuration in plugin-react.
I haven't tested this though, so I don't know if it would really work. :)

[caghand]

With the completion of #9590, babel-plugin-transform-react-createelement-to-jsx has been removed altogether. So, the two problems I reported above no longer exist.
But in my report, I had focused on the automatic runtime, which I was interested in. The isProjectFile problem still exists for people that are not using the automatic runtime.
Do you see the code that adds the @babel/plugin-transform-react-jsx-self and @babel/plugin-transform-react-jsx-source plugins to all project files? With the existing logic, these don't get added to monorepo files outside the project root.

[patak-dev]

@aleclarson would you move this PR to the new repo for plugin-react?