refactor: complete move to Vite Environment API#3634
refactor: complete move to Vite Environment API#3634marvinhagemeister merged 5 commits intodenoland:mainfrom
Conversation
Looked closer, the actual code has a comment saying source maps are always on, so not sure what's up with that? |
| const key = | ||
| `${this.environment.config.consumer}::${resolved.id}::${importer}`; | ||
| if (!seen.has(key)) { | ||
| result = check(pluginOptions, resolved.id, env); | ||
| result = check( | ||
| pluginOptions, | ||
| resolved.id, | ||
| this.environment.config.consumer, | ||
| ); | ||
| } | ||
|
|
||
| seen.add(key); | ||
| } | ||
| } else { | ||
| const key = `${env}::${id}::${importer}`; | ||
| const key = `${this.environment.config.consumer}::${id}::${importer}`; | ||
| if (!seen.has(key)) { | ||
| result = check(pluginOptions, id, env); | ||
| result = check(pluginOptions, id, this.environment.config.consumer); |
There was a problem hiding this comment.
This code might be better off using this.environment.name for the keys.
There was a problem hiding this comment.
Thanks! I'll bring the missing process method up to the CLI team
EDIT: nvm made a patch myself denoland/deno#31358
To make it easy to find misbehaving code in the Vite plugin.
|
canary CI ran with denoland/deno#31358 and still failed, not sure what's up. I think it probably makes sense to split out the ModuleRunner commit, merge the others, and figure out the source map failures separately. |
Filed #3645 for further followup |
2 participants
I was perusing the code whilst investigating #3606 and noticed that, despite the release post claiming that Fresh had "fully embrace[d]" the Vite Environment API, it was still using
ssrLoadModule. Once I started looking, I noticed a bunch of other code that wasn't future-proof (env-wise, not rolldown-wise, which'll bring it's own swath of deprecations1), so I updated them as well.The newly added source map tests from #3624 currently fail again because the ModuleRunner API at the heart of the Environment API relies on
process.setSourceMapsEnabled, which is currently set to no-op in Deno.Footnotes
Separately tried to try out rolldown-vite but it seems that Deno doesn't support aliasing dependencies whatsoever? (I know there's not official support, but I was going to try to edit the lockfile to hack around it; however, it seems that there's not alias support in there at all?) ↩