vite_plugin_deno_resolve icon indicating copy to clipboard operation
vite_plugin_deno_resolve copied to clipboard

npmResolve plugin

Open bartlomieju opened this issue 3 years ago • 9 comments

This PR adds basic support for loading npm: specifiers. It's a quick and dirty way to do that and it doesn't work completely - eg. React is distributed as CommonJS module, while Vite expect that we'll return an ESM module. @itsdouges do you happen to know which plugin can take care of transforming CJS to ESM? It would prefer to avoid having to write that myself as the details are gnarly.

bartlomieju avatar Dec 23 '22 17:12 bartlomieju

Thanks mate! I'm running it locally and getting this error:

Uncaught SyntaxError: ambiguous indirect export: default

Is this because it's resolving to cjs instead of esm?

itsdouges avatar Dec 23 '22 21:12 itsdouges

Thanks mate! I'm running it locally and getting this error:

Uncaught SyntaxError: ambiguous indirect export: default

Is this because it's resolving to cjs instead of esm?

Correct - react distributes as CJS, not ESM. AFAIK by default Vite will prebundle those (https://vitejs.dev/guide/dep-pre-bundling.html), so we should find a way to do the same behavior (or ideally to tell Vite to do that for us instead). I think we might have to implement similar loader for esbuild to teach it where to obtain the sources for various packages when bundling.

bartlomieju avatar Dec 23 '22 21:12 bartlomieju

Right so since we aren't using node modules the whole automatic prebundling step won't work I suppose.

itsdouges avatar Dec 23 '22 21:12 itsdouges

This is almost where I got to with my testing locally. The only difference is id finish by passing the specifier to this.resolve() so it gets the directory resolved by the rollup node resolve plugin. Mostly worked well until you get to the nested entry point case... which without reimplementing everything we'd need Deno info support to give the cache location 🤔

itsdouges avatar Dec 23 '22 21:12 itsdouges

I've made a post on the Vite discord, hopefully someone chimes in https://discord.com/channels/804011606160703521/1056012573713117193/1056012573713117193

itsdouges avatar Dec 24 '22 01:12 itsdouges

Types not loading for the NPM module, what do we need to do to get that working? We want something that works transparently (either through Deno config, or not needing anything at all). A type comment would be a deal breaker IMO.

image image

itsdouges avatar Dec 24 '22 02:12 itsdouges

Types not loading for the NPM module, what do we need to do to get that working? We want something that works transparently (either through Deno config, or not needing anything at all). A type comment would be a deal breaker IMO.

image image

If it works outside Vite project with npm: specifier then that's unexpected, I need to double check with my colleague. That said - comments are needed in some cases now (if a package doesn't provide types configuration in its package.json, automatic acquisition from DefinitelyTyped is not yet implemented).

EDIT: Just checked and it seems react doesn't have types entry right now. So this should be handled as part of https://github.com/denoland/deno/issues/15960

bartlomieju avatar Dec 26 '22 16:12 bartlomieju

Sweet! So the distinction is if the module doesn't ship with types it's a little more complicated and will result in maybe needing the type comment. That's fair enough

itsdouges avatar Dec 26 '22 19:12 itsdouges

Sweet! So the distinction is if the module doesn't ship with types it's a little more complicated and will result in maybe needing the type comment. That's fair enough

Correct, but we'll fix it in the future to automatically try to acquire types from @types/<package_name>

bartlomieju avatar Dec 26 '22 20:12 bartlomieju