npmResolve plugin#3
Conversation
|
Thanks mate! I'm running it locally and getting this error: Is this because it's resolving to cjs instead of esm? |
Correct - |
|
Right so since we aren't using node modules the whole automatic prebundling step won't work I suppose. |
|
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 🤔 |
|
I've made a post on the Vite discord, hopefully someone chimes in https://discord.com/channels/804011606160703521/1056012573713117193/1056012573713117193 |
|
|
||
| export default function npmResolve({}: PluginConfig) { | ||
| export default function npmResolve(config: PluginConfig) { | ||
| // TODO(bartlomieju): both plugins should use the same instance, or maybe |
There was a problem hiding this comment.
It's fine to create two separate ones. The cache instances are passed in from the plugin config in mod.ts.
|
|
||
| const status = await p.status(); | ||
| if (!status.success) { | ||
| throw new Error(`invariant: could not get info on ${name}`); |
| return moduleInfo; | ||
| } | ||
|
|
||
| async function cacheInfo(): Promise<CacheInfo> { |
| ), | ||
| ); | ||
|
|
||
| let file = packageJson.main || 'index.js'; |
There was a problem hiding this comment.
It would be better to pass the package dir here to this.resolve() so the node resolve plugin picks it up and does the work, no?
(Is that what Deno would do internally, basically? Does Deno internally handle exports/main/module/files/folders for entrypoints, etc?
There was a problem hiding this comment.
I'll try, if this.resolve() would work then it would be great. Yes, Deno handles this internally
| let file = packageJson.main || 'index.js'; | ||
| if (ref.subPath) { | ||
| // TODO(bartlomieju): handle other extensions | ||
| file = `${ref.subPath}.js`; |
There was a problem hiding this comment.
This is only one facet of multiple ways a package entrypoint can be defined. If Deno does this internally (I assume it would have to?) perhaps we just focus on getting the Deno side fixed? Else it's a lot of rework here.
There was a problem hiding this comment.
Maybe, but it will be slow - calling a subprocess to resolve each specifier seems worse than porting some code/using a package that can do that.
| versionReq: string | null; | ||
| subPath: string | null; | ||
| } | ||
| function npmPackageReference(specifier: string): NpmPackageReference { |
There was a problem hiding this comment.
It's a direct port from Deno's code, that's how it's handled internally by Deno
|
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.
|
If it works outside Vite project with EDIT: Just checked and it seems |
|
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 |
2 participants
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.