Closed spa5k closed 1 year ago
Hi,
I checkout the repo and tried it to migrate to SWC. I got another issue about cannot resolving react. This make sense because it's used inside your main project without having it as a dev-dependency. Adding react & react-dom as dev-deps works.
Hi,
I checkout the repo and tried it to migrate to SWC. I got another issue about cannot resolving react. This make sense because it's used inside your main project without having it as a dev-dependency. Adding react & react-dom as dev-deps works.
not sure what do you mean by
Adding react & react-dom as dev-deps works.
but its not working
I mean adding dev dependency here: https://github.com/oedotme/generouted/blob/main/packages/generouted/package.json
@vallerydelexy fixed in https://github.com/oedotme/generouted/releases/tag/v1.7.16
Not sure if that fixed the issue with Glob, but i'm still having the main issue with the latest package.
Vite: v4.1.4 @vitejs/plugin-react-swc: v3.2.0 React+-dom: v18.2.0 Generouted: v1.7.16
Can you provide a minimal repro? The first repro was inside the monorepo which is a special case that was fixed
Can you provide a minimal repro? The first repro was inside the monorepo which is a special case that was fixed
Sure, here you go :) Repro link
Thanks for the repro. So the package ships raw TSX with Vite globs into node_modules. For now this plugins does not transpile node modules, that why you get this error. This is a two lines patch if you want to workaround for now. Vite takes care not inluding this into deps prebundling, I will see with the team if we should support this pattern or not
Can you mention the patch for it? I tried optimized deps, but it didn't seem to work.
You need to if (id.includes("node_modules")) return;
This might be something that change in the future, I need to look on how it plays with deps prebundling (on the repo it seems that the file is not part of dependencies pre-bundling)
You need to
if (id.includes("node_modules")) return;
This might be something that change in the future, I need to look on how it plays with deps prebundling (on the repo it seems that the file is not part of dependencies pre-bundling)
Can you be a bit more specific about the patch fix? What do we need to do? Add or remove the highlighted value? And where? In which package? Thanks
You need to find this line in the compiled version (cjs or mjs depending on your package.json): https://github.com/vitejs/vite-plugin-react-swc/blob/main/src/index.ts#L154
any way to get this into the plugin instead of having to patch it?
I believe this plugin should accept includes
and excludes
, this can be achieved by using https://www.npmjs.com/package/@rollup/pluginutils createFilter
method. By default it could filter out node modules while still giving the option to those that need it to effectively override that filter.
It's a very common pattern found in a lot of rollup plugin packages but I do not know your philosophy on adding optionality.
Would this be a good solution @ArnaudBarre ? If so I am able to create the PR to support this as its a feature I am looking for.
For now the philosophy of this plugin is to limit to options and support common future proof pattern out of the box. For example mdx is supported by default.
We will discuss it with the team this Thursday to see if this is a pattern that should be encouragd and maybe better supported by the core (the same question applies for .vue
or .svelte
(svelte plugin allows it but not vue currently)).
Fixed in https://github.com/vitejs/vite-plugin-react-swc/commit/4b9b2d5958adea17398ed569a356821845b55726.
Note that for now this not supported by TS and errors from these files cannot be silenced if the user is using a stricter configuration than the library author: https://github.com/microsoft/TypeScript/issues/30511. I advise to use it only for internal libraries for now.
[plugin:vite:import-glob] Invalid glob import syntax: Expect CallExpression, got BinaryExpression
How to reproduce-