Fix JSR URL template resolution for Homebrew installations#99
Conversation
…ocess The subprocess template resolver failed when running from JSR URLs (e.g., Homebrew installations) because it only accepted file:// modules. This prevented the CLI from working in compiled/remote execution contexts. Changes: - Add getLocalModulePrefix() to determine "local" module prefix by protocol - Support file://, JSR (https://jsr.io/@scope/package/version/), and other URLs - Add comprehensive tests for different URL types This enables Homebrew and other JSR-based distributions to work without pre-embedded templates.
There was a problem hiding this comment.
Pull request overview
This PR fixes JSR URL template resolution to enable the CLI to work from Homebrew installations and other JSR-based distributions. Previously, only file:// URLs were supported for template resolution, which broke when the CLI was run from remote JSR URLs.
Changes:
- Added
getLocalModulePrefix()function to determine "local" modules based on URL protocol (file://, JSR, or other remote URLs) - Modified template resolver to use protocol-aware filtering instead of hardcoded
file://prefix - Added comprehensive unit tests covering different URL types and protocols
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/cli/subprocess_template.ts | Implements protocol-aware local module detection with new getLocalModulePrefix() function and exposes it via _internal for testing |
| src/cli/subprocess_template_test.ts | Adds unit tests covering file://, JSR (HTTP/HTTPS), and other remote URL scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Determine which modules are "local" (should be bundled) | ||
| // For file:// URLs, all file:// modules are local | ||
| // For https:// URLs (JSR), modules under the same package are local | ||
| const localModulePrefix = getLocalModulePrefix(templateUrl); |
There was a problem hiding this comment.
This comment is now outdated since the function handles more than just file:// URLs. The code now also processes JSR URLs and other remote URLs as "local" modules. Consider updating the comment to reflect the broader scope, such as: "Collect all local modules from the graph (based on the module prefix)"
| // For JSR URLs: https://jsr.io/@scope/package/version/... | ||
| // Extract the package base URL to include all modules from the same package | ||
| const jsrMatch = templateUrl.href.match( | ||
| /^(https?:\/\/jsr\.io\/@[^/]+\/[^/]+\/[^/]+\/)/, |
There was a problem hiding this comment.
The regex requires a trailing slash after the version to match JSR URLs. If a JSR URL is provided without a trailing slash and without additional path segments (e.g., https://jsr.io/@scope/package/0.19.0), the regex won't match and the fallback logic will incorrectly strip the version part. While template URLs should typically point to actual files with paths, consider making the trailing slash optional in the regex pattern by changing the last \/ to \/? to handle edge cases more robustly: /^(https?:\/\/jsr\.io\/@[^/]+\/[^/]+\/[^/]+\/?)/
| /^(https?:\/\/jsr\.io\/@[^/]+\/[^/]+\/[^/]+\/)/, | |
| /^(https?:\/\/jsr\.io\/@[^/]+\/[^/]+\/[^/]+\/?)/, |
Summary
getLocalModulePrefix()to determine "local" module prefix based on URL protocolfile://, JSR (https://jsr.io/@scope/package/version/), and other remote URLsWhy
The subprocess template resolver previously only accepted
file://modules, causing failures when the CLI was run from JSR URLs (e.g., Homebrew installations). This prevented the CLI from working in compiled or remote execution contexts.The fix introduces protocol-aware "local module" detection:
file://): Bundles all local file moduleshttps://jsr.io/...): Bundles all modules within the same packageThis enables Homebrew and other JSR-based distributions to work without requiring pre-embedded templates.
Test Plan
getLocalModulePrefix()with various URL typesdeno task verifypasses)