itsdouges / vite_plugin_deno_resolve

A plugin for Vite that resolves modules with Deno.
https://deno.land/x/vite_plugin_deno_resolve
16 stars 3 forks source link

npmResolve plugin #3

Open bartlomieju opened 1 year ago

bartlomieju commented 1 year ago

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.

itsdouges commented 1 year ago

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?

bartlomieju commented 1 year ago

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.

itsdouges commented 1 year ago

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

itsdouges commented 1 year ago

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 commented 1 year ago

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

itsdouges commented 1 year ago

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
bartlomieju commented 1 year ago

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

itsdouges commented 1 year ago

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

bartlomieju commented 1 year ago

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>