Support TS/JSX inside node_modules

[spa5k]

[plugin:vite:import-glob] Invalid glob import syntax: Expect CallExpression, got BinaryExpression

F:/app/node_modules/.pnpm/generouted@1.7.2_vite@4.1.1/node_modules/generouted/src/react-location.tsx:9:19

7  |  type Module = { default: Element; Loader: LoaderFn; PendingElement: Element; ErrorElement: Element }
8  |  
9  |  const PRESERVED = import.meta.glob<Module>('/src/pages/(_app|404).{jsx,tsx}', { eager: true })
   |                    ^
10 |  const ROUTES = import.meta.glob<Module>(['/src/pages/**/[\\w[]*.{jsx,tsx}', '!**/(_app|404).*'])
11 |

How to reproduce-

1. Clone this example - https://github.com/oedotme/generouted/tree/main/examples/react-location/nested-layouts
2. Update vite config to use plugin-react-swc rather than the original one, you will get the error.

[ArnaudBarre]

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.

[vallerydelexy]

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

[ArnaudBarre]

I mean adding dev dependency here: https://github.com/oedotme/generouted/blob/main/packages/generouted/package.json

[spa5k]

@vallerydelexy fixed in https://github.com/oedotme/generouted/releases/tag/v1.7.16

[HarunKilic]

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

[ArnaudBarre]

Can you provide a minimal repro?
The first repro was inside the monorepo which is a special case that was fixed

[HarunKilic]

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

[ArnaudBarre]

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

[spa5k]

Can you mention the patch for it? I tried optimized deps, but it didn't seem to work.

[ArnaudBarre]

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)

[HarunKilic]

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

[ArnaudBarre]

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

[opeologist]

any way to get this into the plugin instead of having to patch it?

[TomCaserta]

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.

[ArnaudBarre]

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)).

[ArnaudBarre]

Fixed in 4b9b2d5.

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: microsoft/TypeScript#30511. I advise to use it only for internal libraries for now.