feat: get version from deno.json instead of fetching latest#1046
feat: get version from deno.json instead of fetching latest#1046
Conversation
Tagging OptionsShould a new tag be published when this PR is merged?
|
📝 WalkthroughWalkthroughThe pull request modifies Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="scripts/run.ts">
<violation number="1" location="scripts/run.ts:20">
P2: The regex captures the last `@` segment even when the specifier has no version (e.g., `jsr:@deco/deco`), resulting in `version = "deco/deco"` and an invalid import. Use a pattern that only captures when a version `@...` is present after the package name.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| // Extract version from jsr:@deco/deco@1.133.2 or jsr:@deco/dev@1.133.4 | ||
| const match = specifier.match(/@([^@]+)$/); |
There was a problem hiding this comment.
P2: The regex captures the last @ segment even when the specifier has no version (e.g., jsr:@deco/deco), resulting in version = "deco/deco" and an invalid import. Use a pattern that only captures when a version @... is present after the package name.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/run.ts, line 20:
<comment>The regex captures the last `@` segment even when the specifier has no version (e.g., `jsr:@deco/deco`), resulting in `version = "deco/deco"` and an invalid import. Use a pattern that only captures when a version `@...` is present after the package name.</comment>
<file context>
@@ -1,29 +1,37 @@
+ }
+
+ // Extract version from jsr:@deco/deco@1.133.2 or jsr:@deco/dev@1.133.4
+ const match = specifier.match(/@([^@]+)$/);
+ if (!match) {
+ console.warn("Could not parse version from specifier, falling back to major version");
</file context>
| const match = specifier.match(/@([^@]+)$/); | |
| const match = specifier.match(/@[^@]+@([^/]+)$/); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/run.ts`:
- Around line 11-27: The code is failing to parse JSR specifiers and still uses
a hardcoded package name; update the parsing to extract both package and version
from the found specifier (the variable specifier) using a pattern like
/^jsr:([^@]+)@(.+)$/ so you get packageName = match[1] and version = match[2].
Strip a leading ^ from version as you already do, log the version, and use the
parsed packageName (not the hardcoded "@deco/deco") when performing the dynamic
import or any further resolution; ensure checks still handle missing/invalid
specifier by falling back to the major version "1".
🧹 Nitpick comments (1)
scripts/run.ts (1)
7-9: Consider supporting JSONC format for better robustness (optional improvement).The code already handles JSON parse errors gracefully with a try-catch fallback, but if
deno.jsonever uses comments or trailing commas, or if the repo switches todeno.jsonc, the fallback to version "1" will silently occur. For more robust version detection, consider using a JSONC parser and/or checkingdeno.jsoncas an alternative whendeno.jsonis missing.
| // Try @deco/deco first, then @deco/dev | ||
| const specifier = imports["@deco/deco"] ?? imports["@deco/dev"]; | ||
|
|
||
| if (!specifier || !specifier.startsWith("jsr:")) { | ||
| console.warn("No jsr specifier found, falling back to major version"); | ||
| return "1"; | ||
| } | ||
|
|
||
| // Extract version from jsr:@deco/deco@1.133.2 or jsr:@deco/dev@1.133.4 | ||
| const match = specifier.match(/@([^@]+)$/); | ||
| if (!match) { | ||
| console.warn("Could not parse version from specifier, falling back to major version"); | ||
| return "1"; | ||
| } | ||
|
|
||
| const version = match[1].replace(/^\^/, ""); // Remove ^ if present | ||
| console.log(`%cusing version ${version} from deno.json`, "color: gray"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n scripts/run.tsRepository: deco-cx/deco
Length of output: 1583
🏁 Script executed:
cat -n deno.jsonRepository: deco-cx/deco
Length of output: 4414
🏁 Script executed:
# Check for deno.jsonc or other deno config files
fd -e jsonc -e json | grep -i denoRepository: deco-cx/deco
Length of output: 97
🏁 Script executed:
# Search for `@deco/deco` or `@deco/dev` usage in deno.json* files
rg -n --glob 'deno.json*' '@deco/(deco|dev)' || echo "No matches found"Repository: deco-cx/deco
Length of output: 200
🏁 Script executed:
# Test the regex pattern against potential specifiers
python3 << 'EOF'
import re
regex = r"/@([^@]+)$"
test_cases = [
"jsr:`@deco/deco`@1.133.2",
"jsr:`@deco/dev`@1.133.4",
"jsr:`@deco/deco`@1.2.3/subpath",
"jsr:`@deco/deco`@^1.2.3",
"jsr:`@deco/deco`",
]
for spec in test_cases:
match = re.search(regex, spec)
if match:
print(f"✓ {spec:35} → captures: '{match.group(1)}'")
else:
print(f"✗ {spec:35} → NO MATCH")
EOFRepository: deco-cx/deco
Length of output: 300
Fix package name resolution and JSR specifier parsing.
The current implementation has critical bugs:
-
Broken regex: The pattern
/@([^@]+)$/requires a/before the final@, but JSR specifiers likejsr:@deco/deco@1.133.2don't have this format. The regex will never match, causing the function to always return the fallback version "1". -
Package name mismatch: The hardcoded
packageName = "@deco/deco"on line 3 is never updated based on which specifier is actually found on line 12. Ifimports["@deco/dev"]is configured, the code will still import@deco/deco.
Extract both the package name and version from the specifier and use the actually-found package name in the dynamic import.
Proposed fix
-const packageName = "@deco/deco";
+const DEFAULT_PACKAGE = "@deco/deco";
-const getVersionFromDenoJson = async (): Promise<string> => {
+const getPackageAndVersionFromDenoJson = async (): Promise<{
+ packageName: string;
+ version: string;
+}> => {
try {
const denoJsonPath = `${Deno.cwd()}/deno.json`;
const denoJson = JSON.parse(await Deno.readTextFile(denoJsonPath));
const imports = denoJson.imports ?? {};
// Try `@deco/deco` first, then `@deco/dev`
const specifier = imports["@deco/deco"] ?? imports["@deco/dev"];
if (!specifier || !specifier.startsWith("jsr:")) {
console.warn("No jsr specifier found, falling back to major version");
- return "1";
+ return { packageName: DEFAULT_PACKAGE, version: "1" };
}
- // Extract version from jsr:`@deco/deco`@1.133.2 or jsr:`@deco/dev`@1.133.4
- const match = specifier.match(/@([^@]+)$/);
+ // Extract package + version from jsr:`@deco/deco`@1.133.2
+ const match = specifier.match(/^jsr:(@[^@/]+\/[^@/]+)@([^/]+)(?:\/.*)?$/);
if (!match) {
console.warn("Could not parse version from specifier, falling back to major version");
- return "1";
+ return { packageName: DEFAULT_PACKAGE, version: "1" };
}
- const version = match[1].replace(/^\^/, ""); // Remove ^ if present
+ const [, packageName, rawVersion] = match;
+ const version = rawVersion.replace(/^\^/, ""); // Remove ^ if present
console.log(`%cusing version ${version} from deno.json`, "color: gray");
- return version;
+ return { packageName, version };
} catch (err) {
console.warn("Could not read deno.json", err, "falling back to major version");
- return "1";
+ return { packageName: DEFAULT_PACKAGE, version: "1" };
}
};
-const version = await getVersionFromDenoJson();
+const { packageName, version } = await getPackageAndVersionFromDenoJson();
await import(`jsr:${packageName}@${version}/scripts/run`);🤖 Prompt for AI Agents
In `@scripts/run.ts` around lines 11 - 27, The code is failing to parse JSR
specifiers and still uses a hardcoded package name; update the parsing to
extract both package and version from the found specifier (the variable
specifier) using a pattern like /^jsr:([^@]+)@(.+)$/ so you get packageName =
match[1] and version = match[2]. Strip a leading ^ from version as you already
do, log the version, and use the parsed packageName (not the hardcoded
"@deco/deco") when performing the dynamic import or any further resolution;
ensure checks still handle missing/invalid specifier by falling back to the
major version "1".
1 participant
Summary
Test plan
Summary by cubic
Use the @deco/deco or @deco/dev version from deno.json imports instead of fetching the latest from JSR. This aligns the run script with the project's configured version and removes a network dependency.
Written for commit 6d4af22. Summary will update on new commits.
Summary by CodeRabbit