[cli] add pinned deps support to mops update and mops outdated#316
Conversation
WalkthroughAdds alias support to add(), introduces pinned-version handling across available-updates, outdated display, and update flows, and adds a helper to parse pinned versions. Update now resolves dev/alias by matching base names and compatible pins and passes alias to add(). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI as CLI update
participant AU as available-updates
participant Cfg as Config
participant Add as add()
User->>CLI: mops update
CLI->>Cfg: Load dependencies & dev-dependencies
CLI->>AU: getAvailableUpdates(config)
AU->>Cfg: Read deps with pins
AU-->>CLI: [[name, currentVersion, updateVersion]] (pin-aware)
CLI->>CLI: Resolve dev & asName via getDepName/getDepPinnedVersion
loop For each update
CLI->>Add: add("name@updateVersion", {dev}, asName)
Add->>Cfg: Apply install using asName (if provided)
end
CLI-->>User: Update summary (aliases respected)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Poem
✨ Finishing Touches
🧪 Generate unit tests
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 |
Benchmark Resultsbench/1-buffer-vector-add.bench.mo
|
| 10 | 10000 | 1000000 | |
|---|---|---|---|
| Buffer | 9_493 |
5_687_530 |
525_783_824 |
| Vector | 13_461 |
4_378_548 |
417_864_434 |
Heap
| 10 | 10000 | 1000000 | |
|---|---|---|---|
| Buffer | 272 B |
272 B |
272 B |
| Vector | 272 B |
272 B |
272 B |
Garbage Collection
| 10 | 10000 | 1000000 | |
|---|---|---|---|
| Buffer | 1.09 KiB |
143.28 KiB |
12.02 MiB |
| Vector | 1.09 KiB |
45.65 KiB |
3.86 MiB |
bench/2-vector-buffer-add.bench.mo $({\color{green}-10.20\%})$
Add
Add items one-by-one
Instructions:
Heap:
Stable Memory:
Garbage Collection:
Instructions
| 10 | 10000 | 1000000 | |
|---|---|---|---|
| Vector | 13_461 |
4_378_902 |
417_886_028 |
| Buffer | 9_493 |
5_686_822 |
525_780_992 |
Heap
| 10 | 10000 | 1000000 | |
|---|---|---|---|
| Vector | 272 B |
272 B |
272 B |
| Buffer | 272 B |
272 B |
272 B |
Garbage Collection
| 10 | 10000 | 1000000 | |
|---|---|---|---|
| Vector | 1.09 KiB |
45.65 KiB |
3.86 MiB |
| Buffer | 1.09 KiB |
143.28 KiB |
12.02 MiB |
bench/array.bench.mo $({\color{green}-18.10\%})$
Array
arr arr
Instructions:
Heap:
Stable Memory:
Garbage Collection:
Instructions
| 100k x1 | reset1 | 100k x3 | reset2 | 100k x4 | reset3 | |
|---|---|---|---|---|---|---|
| Array | 13_502_032 |
3_271 |
27_003_206 |
3_745 |
54_004_063 |
4_219 |
Heap
| 100k x1 | reset1 | 100k x3 | reset2 | 100k x4 | reset3 | |
|---|---|---|---|---|---|---|
| Array | 390.9 KiB |
-390.37 KiB |
390.9 KiB |
-390.37 KiB |
390.9 KiB |
-390.37 KiB |
Garbage Collection
| 100k x1 | reset1 | 100k x3 | reset2 | 100k x4 | reset3 | |
|---|---|---|---|---|---|---|
| Array | 360 B |
390.97 KiB |
391 KiB |
390.97 KiB |
1.14 MiB |
390.97 KiB |
bench/removeLast.bench.mo $({\color{green}-10.90\%})$
Remove items using removeLast
Vector and buffer are initialized with 100k items and then 70k items are removed one-by-one.
Instructions:
Heap:
Stable Memory:
Garbage Collection:
Instructions
| remove 70k | |
|---|---|
| Vector | 27_707_652 |
| Buffer | 29_236_913 |
Heap
| remove 70k | |
|---|---|
| Vector | -136.8 KiB |
| Buffer | -269.76 KiB |
Garbage Collection
| remove 70k | |
|---|---|
| Vector | 139.45 KiB |
| Buffer | 540.43 KiB |
bench/stable-memory.bench.mo $({\color{green}-134.72\%})$
Stable Memory and Region
Grow Region and store blobs in it
Instructions:
Heap:
Stable Memory:
Garbage Collection:
Instructions
| Region (fill 1/100) | Region (fill 1/50) | StableMemory | |
|---|---|---|---|
| 10 pages | 2_626_925 |
10_496_350 |
2_629 |
| 100 pages | 52_466_897 |
104_914_490 |
2_634 |
| 256 pages | 134_273_534 |
268_574_687 |
3_182 |
Heap
| Region (fill 1/100) | Region (fill 1/50) | StableMemory | |
|---|---|---|---|
| 10 pages | 272 B |
272 B |
276 B |
| 100 pages | 272 B |
272 B |
272 B |
| 256 pages | 272 B |
272 B |
276 B |
Garbage Collection
| Region (fill 1/100) | Region (fill 1/50) | StableMemory | |
|---|---|---|---|
| 10 pages | 208.34 KiB |
832.38 KiB |
336 B |
| 100 pages | 4.06 MiB |
8.13 MiB |
340 B |
| 256 pages | 10.4 MiB |
20.8 MiB |
340 B |
Stable Memory
| Region (fill 1/100) | Region (fill 1/50) | StableMemory | |
|---|---|---|---|
| 10 pages | 8 MiB |
8 MiB |
8 MiB |
| 100 pages | 8 MiB |
8 MiB |
0 B |
| 256 pages | 16 MiB |
16 MiB |
16 MiB |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/commands/update.ts (1)
20-23:pkgcheck rejects valid installs when user passes base name (e.g.,coreforcore@1).Accept base names by mapping to the installed key before validating.
- if (pkg && !config.dependencies?.[pkg] && !config['dev-dependencies']?.[pkg]) { - console.log(chalk.red(`Package "${pkg}" is not installed!`)); - return; - } + let pkgKey = pkg; + if (pkg) { + const depKeys = [ + ...Object.keys(config.dependencies || {}), + ...Object.keys(config['dev-dependencies'] || {}), + ]; + const exact = depKeys.includes(pkg); + const byBase = depKeys.find(d => getDepName(d) === pkg); + if (!exact && !byBase) { + console.log(chalk.red(`Package "${pkg}" is not installed!`)); + return; + } + pkgKey = exact ? pkg : byBase!; + }
🧹 Nitpick comments (11)
cli/commands/add.ts (1)
63-67: Avoid re-parsing GitHub URL.You already have
gitNamefrom Line 52; reuse it instead of callingparseGithubURL(name)again.- pkgDetails = { - name: asName || parseGithubURL(name).gitName, + pkgDetails = { + name: asName || gitName, repo: `https://github.com/${org}/${gitName}#${branch}@${commitHash}`, version: '', };cli/helpers/get-dep-name.ts (2)
5-6: Provide a boundary-safe pin matcher to avoid prefix false-positives.
startsWithon pins (e.g.,'1'matching'10.0.0') is unsafe. Centralize matching here and reuse.export function getDepPinnedVersion(name : string) : string { return name.split('@')[1] || ''; } + +// Matches major/minor/patch pins without prefix false-positives. +// Examples: +// pin "1" -> matches 1.x.x only +// pin "1.2" -> matches 1.2.x only +// pin "1.2.3" -> matches exactly 1.2.3 +export function matchesPinnedVersion(version: string, pin: string): boolean { + if (!pin) return true; + const v = version.split('.'); + const p = pin.split('.'); + if (p.length === 1) return v[0] === p[0]; + if (p.length === 2) return v[0] === p[0] && v[1] === p[1]; + return version === pin; +}
1-3: Potential edge-case if package names can contain '@'.If names may legitimately include '@' (scopes),
split('@')will misparse. ConsiderlastIndexOf('@')or a stricter pin syntax.cli/commands/outdated.ts (1)
19-27: Pin check can mis-match majors (e.g., '1' vs '10').Replace
startsWithwith a boundary-safe matcher.-import {getDepName, getDepPinnedVersion} from '../helpers/get-dep-name.js'; +import {getDepName, getDepPinnedVersion, matchesPinnedVersion} from '../helpers/get-dep-name.js'; ... - let name = allDeps.find((d) => { - let pinnedVersion = getDepPinnedVersion(d); - return getDepName(d) === dep[0] && (!pinnedVersion || dep[1].startsWith(pinnedVersion)); - }) || dep[0]; + let name = allDeps.find((d) => { + const pin = getDepPinnedVersion(d); + return getDepName(d) === dep[0] && matchesPinnedVersion(dep[1], pin); + }) || dep[0];cli/commands/update.ts (5)
26-33: Ensure GitHub filtering also respects base-name invocation.When
pkgis provided as a base name, include matches by base for GitHub deps.- if (pkg) { - githubDeps = githubDeps.filter((dep) => dep.name === pkg); - } + if (pkgKey) { + githubDeps = githubDeps.filter((dep) => dep.name === pkgKey || getDepName(dep.name) === pkg); + }
42-44: Pass the resolved package key to updates retrieval.Aligns with the relaxed
pkghandling.- let available = await getAvailableUpdates(config, pkg); + let available = await getAvailableUpdates(config, pkgKey);
6-6: Import the boundary-safe matcher.Required by the changes below.
-import {getDepName, getDepPinnedVersion} from '../helpers/get-dep-name.js'; +import {getDepName, getDepPinnedVersion, matchesPinnedVersion} from '../helpers/get-dep-name.js';
55-73: ReplacestartsWithwith boundary-safe pin matching.Prevents '1' matching '10.x' and keeps dev/alias resolution correct.
- let devDeps = Object.keys(config['dev-dependencies'] || {}); - let allDeps = [...Object.keys(config.dependencies || {}), ...devDeps]; + let devDeps = Object.keys(config['dev-dependencies'] || {}); + let allDeps = [...Object.keys(config.dependencies || {}), ...devDeps]; let dev = false; for (let d of devDeps) { - let pinnedVersion = getDepPinnedVersion(d); - if (getDepName(d) === dep[0] && (!pinnedVersion || dep[1].startsWith(pinnedVersion))) { + const pin = getDepPinnedVersion(d); + if (getDepName(d) === dep[0] && matchesPinnedVersion(dep[1], pin)) { dev = true; break; } } - let asName = allDeps.find((d) => { - let pinnedVersion = getDepPinnedVersion(d); - return getDepName(d) === dep[0] && (!pinnedVersion || dep[1].startsWith(pinnedVersion)); - }) || dep[0]; + let asName = allDeps.find((d) => { + const pin = getDepPinnedVersion(d); + return getDepName(d) === dep[0] && matchesPinnedVersion(dep[1], pin); + }) || dep[0];
55-57: Minor: variable shadowing can confuse readers.
devDepsis used earlier as values; here it’s keys. ConsiderdevDepKeys.cli/commands/available-updates.ts (2)
32-40: Deriving SemverPart via split length is fragile; use regex.Covers
1,1.2,1.x/1.*, and ignores hard pins already filtered out.Apply:
- let res = await actor.getHighestSemverBatch(depsToUpdate.map((dep) => { - let semverPart : SemverPart = {major: null}; - let name = getDepName(dep.name); - let pinnedVersion = getDepPinnedVersion(dep.name); - if (pinnedVersion) { - semverPart = pinnedVersion.split('.').length === 1 ? {minor: null} : {patch: null}; - } - return [name, dep.version || '', semverPart]; - })); + let res = await actor.getHighestSemverBatch(depsToUpdate.map((dep) => { + const name = getDepName(dep.name); + const pin = normalizePin(getDepPinnedVersion(dep.name)); + let semverPart: SemverPart = { major: null }; + if (pin) { + if (/^\d+$/.test(pin) || /^\d+\.(?:x|\*)$/.test(pin)) semverPart = { minor: null }; + else if (/^\d+\.\d+$/.test(pin) || /^\d+\.\d+\.(?:x|\*)$/.test(pin)) semverPart = { patch: null }; + } + return [name, dep.version || '', semverPart]; + }));
47-47: Don’t emit rows with empty current version; compute once.Prevents
[name, '', new]when a pin mismatch occurs; avoids double traversal.Apply:
-return res.ok.filter((dep) => dep[1] !== getCurrentVersion(dep[0], dep[1])).map((dep) => [dep[0], getCurrentVersion(dep[0], dep[1]), dep[1]]); +return res.ok + .map(([name, next]) => { + const curr = getCurrentVersion(name, next); + return [name, curr, next] as [string, string, string]; + }) + .filter(([, curr, next]) => !!curr && curr !== next);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cli/commands/add.ts(1 hunks)cli/commands/available-updates.ts(2 hunks)cli/commands/outdated.ts(2 hunks)cli/commands/update.ts(2 hunks)cli/helpers/get-dep-name.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
cli/commands/outdated.ts (1)
cli/helpers/get-dep-name.ts (2)
getDepPinnedVersion(5-7)getDepName(1-3)
cli/commands/update.ts (2)
cli/helpers/get-dep-name.ts (2)
getDepPinnedVersion(5-7)getDepName(1-3)cli/commands/add.ts (1)
add(22-134)
cli/commands/available-updates.ts (2)
cli/helpers/get-dep-name.ts (2)
getDepName(1-3)getDepPinnedVersion(5-7)cli/declarations/main/main.did.d.ts (1)
SemverPart(302-304)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: node 20, moc 0.14.13, ic-mops@1.0.0
- GitHub Check: ubuntu-latest, node 20, moc 0.14.13, ic-mops
- GitHub Check: ubuntu-latest, node 24, moc 0.14.13, ic-mops
- GitHub Check: ubuntu-latest, node 20, moc 0.14.13, ./cli/dist
- GitHub Check: ubuntu-latest, node 24, moc 0.14.13, ./cli/dist
- GitHub Check: Run Benchmarks and Comment
- GitHub Check: Test with ZenVoich/setup-mops
🔇 Additional comments (5)
cli/commands/add.ts (1)
86-89: Alias application for Mops packages — looks good.Using
asName || nameensures aliased keys (e.g.,core@1) are preserved in config during installs/updates.cli/commands/update.ts (2)
72-73: Sanity-check: preserve alias when updating.Passing
asNameintoaddis correct and keepscore@1keys stable.
14-76: Edge-case: multiple aliases (prod/dev) for the same base.If both
core@1(prod) andcore@1(dev) exist, resolution may pick the first match. Consider resolving by the exact key whenpkgis specified and by unique match otherwise; if ambiguous, prompt.Do you want me to add a follow-up patch that disambiguates and warns when multiple aliases match a base?
cli/commands/available-updates.ts (2)
5-6: The verification scripts are running; I’ll review the results and update the comment accordingly.
5-6: Validate DID import and output tuple shape
cli/declarations/main/main.did.jsexists, so importingSemverPartfrom its.jsfile is valid; useimport type { SemverPart }if it’s only needed at compile time.- The
getHighestSemverBatchmethod returnsArray<[PackageName, PackageVersion, SemverPart]>(percli/declarations/main/main.did.d.ts), so ensure your code destructures and handles the thirdSemverPartelement in the returned tuples.
| // skip hard pinned dependencies (e.g. "base@X.Y.Z") | ||
| depsToUpdate = depsToUpdate.filter((dep) => getDepName(dep.name) === dep.name || getDepPinnedVersion(dep.name).split('.').length !== 3); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hard-pin detection is brittle (pre-release/build tags misclassified).
split('.') misses cases like 1.2.3-rc.1 and may incorrectly update hard-pinned deps. Parse with a regex and normalize the pin.
Apply:
-// skip hard pinned dependencies (e.g. "base@X.Y.Z")
-depsToUpdate = depsToUpdate.filter((dep) => getDepName(dep.name) === dep.name || getDepPinnedVersion(dep.name).split('.').length !== 3);
+// skip hard-pinned dependencies (e.g. "base@X.Y.Z" or "base@X.Y.Z-rc.1")
+depsToUpdate = depsToUpdate.filter((dep) => {
+ const pin = normalizePin(getDepPinnedVersion(dep.name));
+ const hardPinned = /^\d+\.\d+\.\d+(?:[-+].*)?$/.test(pin);
+ return !hardPinned;
+});Outside selected lines: also make pkg filtering pin/alias-aware, otherwise mops update core won’t select core@1:
- let depsToUpdate = pkg ? allDeps.filter((dep) => dep.name === pkg) : allDeps;
+ const pkgBase = pkg ? getDepName(pkg) : '';
+ let depsToUpdate = pkg ? allDeps.filter((dep) => getDepName(dep.name) === pkgBase) : allDeps;Support code to add near the top of the file:
function normalizePin(pin: string): string {
return pin.trim().replace(/^v/i, '');
}| let getCurrentVersion = (pkg : string, updateVersion : string) => { | ||
| for (let dep of allDeps) { | ||
| if (dep.name === pkg && dep.version) { | ||
| if (getDepName(dep.name) === pkg && dep.version) { | ||
| let pinnedVersion = getDepPinnedVersion(dep.name); | ||
| if (pinnedVersion && !updateVersion.startsWith(pinnedVersion)) { | ||
| continue; | ||
| } | ||
| return dep.version; | ||
| } | ||
| } | ||
| return ''; | ||
| }; |
There was a problem hiding this comment.
Pin check using startsWith() is wrong (e.g., pin "1" matches "10.x").
This can enable invalid updates. Use boundary-aware matching and exclude rows when the pin doesn’t match.
Apply:
-let getCurrentVersion = (pkg : string, updateVersion : string) => {
+let getCurrentVersion = (pkg : string, updateVersion : string) => {
for (let dep of allDeps) {
- if (getDepName(dep.name) === pkg && dep.version) {
- let pinnedVersion = getDepPinnedVersion(dep.name);
- if (pinnedVersion && !updateVersion.startsWith(pinnedVersion)) {
- continue;
- }
- return dep.version;
- }
+ if (!dep.version) continue;
+ if (getDepName(dep.name) !== pkg) continue;
+ const pin = normalizePin(getDepPinnedVersion(dep.name));
+ if (pin && !matchesPin(pin, updateVersion)) continue;
+ return dep.version;
}
return '';
};Support code to add once (near normalizePin):
function matchesPin(pin: string, version: string): boolean {
const v = normalizePin(version);
if (/^\d+$/.test(pin)) return new RegExp(`^${pin}\\.\\d+\\.\\d+(?:[-+].*)?$`).test(v);
if (/^\d+\\.\d+$/.test(pin)) return new RegExp(`^${pin}\\.\\d+(?:[-+].*)?$`).test(v);
if (/^\d+\\.\d+\\.\d+(?:[-+].*)?$/.test(pin)) return new RegExp(`^${pin}(?:$|[-+])`).test(v);
return true;
}🤖 Prompt for AI Agents
In cli/commands/available-updates.ts around lines 18 to 29, the pin check
currently uses startsWith() which incorrectly matches pins like "1" to versions
"10.x"; replace that logic with a boundary-aware matcher: add the provided
matchesPin(pin, version) helper (place it once near normalizePin) and use
matchesPin(pinnedVersion, dep.version) instead of
updateVersion.startsWith(pinnedVersion); if matchesPin returns false, skip that
dep (exclude the row). Ensure the helper handles integer, "major.minor", and
full "major.minor.patch" pin formats as described so pins only match proper
version boundaries.
1 participant
fixes #315
Summary by CodeRabbit
New Features
Improvements