Conversation
Author
|
Note: the lint-and-format check is failing, but this is a pre-existing failure on This PR fixes #40. |
Author
|
@marvinhagemeister could you ptal? |
Author
|
@bartlomieju could you ptal? this has been a problem that trips us every time |
uriva
force-pushed
the
fix/npm-specifier-resolution
branch
from
September 13, 2026 21:31
15b4eb9 to
f42e4dd
Compare
Two bugs in prefixPlugin's npm: specifier handling:
1. Scoped package names (e.g. @preact/signals@2.8.1_preact@10.25.4)
were parsed incorrectly: indexOf('@') returns 0 for '@preact/...',
resulting in an empty package name. Fixed by finding the version
separator '@' after the scope's '/'.
2. When this.resolve() fails (package not in node_modules), the bare
package name was returned as-is, causing Vite to silently
externalize it. The error only surfaced at runtime in the browser.
Now emits a warning and returns undefined so Vite can report the
unresolved import at build time.
Closes denoland#40
uriva
force-pushed
the
fix/npm-specifier-resolution
branch
from
September 13, 2026 21:33
f42e4dd to
57fb984
Compare
uriva
force-pushed
the
fix/npm-specifier-resolution
branch
from
September 13, 2026 21:36
4327ffe to
41c06b5
Compare
Contributor
|
I'm afraid I can't. I don't work for Deno anymore and don't have any maintainer permissions anymore. Please contact other members of the Deno team. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two bugs in
prefixPlugin.tswhen resolvingnpm:specifiers. Closes #40.Bug 1: Scoped package names parsed incorrectly
deno info --jsonreturnsnpmPackageidentifiers like:preact@10.25.4@preact/signals@2.8.1_preact@10.25.4The existing code uses
indexOf("@")to find the version separator. For scoped packages starting with@, this returns0, soslice(0, 0)produces an empty string"". The fix finds the@after the/in scoped package names.Bug 2: Silent externalization on resolution failure
When
this.resolve(packageName)fails (package not innode_modules), the old code returned the bare package name as a string. Vite interpreted this as an external module and silently externalized it — the error only appeared at runtime in the browser as:Now the plugin emits a warning with actionable guidance and returns
undefined, allowing Vite to report the unresolved import at build time instead.Why packages might not be in node_modules
This commonly happens when an
npm:specifier comes from a JSR package. Deno resolves JSR transitive npm dependencies through its own module graph, but they don't get installed into the project'snode_modules/. The warning tells users to add the package to their import map orpackage.json.Tests
extractPackageNameunit tests covering unscoped, scoped, scoped with peer dep suffixes, and versionless package identifiersnpm:@preact/signals(scoped package)