Skip to content

fix: fall back to package.json for CLI version detection #21382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PhilWindle merged 1 commit into from
Mar 12, 2026
Merged

fix: fall back to package.json for CLI version detection #21382

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions yarn-project/stdlib/src/update-checker/package_version.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,28 @@ import { fileURLToPath } from '@aztec/foundation/url';
import { readFileSync } from 'fs';
import { dirname, resolve } from 'path';

/** Returns the package version from the release-please manifest, or undefined if not found. */
/** Returns the package version from the release-please manifest or the package.json, or undefined if not found. */
export function getPackageVersion(): string | undefined {
const dir = dirname(fileURLToPath(import.meta.url));

// Try the release-please manifest first (works in dev/repo checkout).
try {
const releasePleaseManifestPath = resolve(
dirname(fileURLToPath(import.meta.url)),
'../../../../.release-please-manifest.json',
);
const releasePleaseManifestPath = resolve(dir, '../../../../.release-please-manifest.json');
return JSON.parse(readFileSync(releasePleaseManifestPath).toString())['.'];
} catch {
return undefined;
// Not in a repo checkout, fall through.
}

// Fall back to the stdlib package.json version (works in npm-installed packages).
try {
const packageJsonPath = resolve(dir, '../../package.json');
const version = JSON.parse(readFileSync(packageJsonPath).toString()).version;
if (version && version !== '0.1.0') {
return version;
}
} catch {
// No package.json found either.

@benesjan benesjan May 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ludamad I would ping you about this on slack but at this point I don't know how so asking here.

2 things seem strange to me in this function:

  1. We are getting the version here from .release-please-manifest.json which seems strange given that release please is deprecated. Also it doesn't seem to correctly resolve the specific version (e.g. a given nightly) and instead it just resolves the future stable version (e.g. currently 4.3.0 on v4).
  2. When we don't find the package.json we don't error out and instead we just return undefined.

Given these 2 problems I implemented my own function in this PR that looks as follows:

/**
 * Returns the package version for embedding in contract artifacts or `dev` if the version in package.json is
 * the placeholder value (that would indicate that this is used from a locally checked out monorepo instead of npm).
 */
function getPackageVersionOrDev(): string {
  const dir = dirname(fileURLToPath(import.meta.url));
  const packageJsonPath = resolve(dir, '../../../package.json');
  const version = JSON.parse(readFileSync(packageJsonPath).toString()).version;
  return version === '0.1.0' ? DEV_VERSION : version;
}

What this does is that it returns the version from package.json or "dev" string when the code is run froms a checked out monorepo and not a real release.

I think we should replace the getPackageVersion function with my implementation.

(and maybe drop the .release-please-manifest.json file)

Would you agree?

@benesjan benesjan May 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI feedback

Agree with the diagnosis, broadly agree with the replacement, but a few wrinkles before wholesale replacing it.

On the diagnosis — yes:

  • .release-please-manifest.json currently holds 5.0.0 in this checkout. That's the next stable target, not the actually-installed/built version. So for nightlies and stable releases the manifest reading is straight-up wrong; package.json (which release tooling stamps at publish time) is the right source.
  • Returning undefined forces every caller into ?? '<fallback>' ('', 'unknown', '0.0.0' — all different, none ideal). A typed string is cleaner.

Wrinkles to think about before swapping:

  1. Path depth. Your snippet uses ../../../package.json but the existing fallback in this file uses ../../package.json. For dest/update-checker/package_version.js in the published stdlib package, ../../ is the package root. Make sure the depth in feat: stamping aztec version into contract artifacts #22550 matches the file's actual location, not a copy-paste from elsewhere.

  2. Caller behavior change in dev. Today, in a repo checkout, every caller sees 5.0.0. After the change they'll see 'dev'. Mostly fine, but warn_if_aztec_version_mismatch.ts builds expectedTag = \v${version}`→'vdev'and will then warn on every aztec-nr dep. That caller needs an explicitif (version === DEV_VERSION) return;` guard.

  3. Other callers of ?? ''. aztec-node/server.ts etc. compare versions across the network — if the node reports 'dev', downstream comparisons should treat that explicitly rather than as "just another version string." Worth a pass.

  4. Don't delete .release-please-manifest.json in this PR. Even if release-please is deprecated, that file may still be consumed by current release tooling / GH actions. Confirm it's actually unused first; it's a separate, riskier change from "stop reading it from JS."

  5. Naming. getPackageVersionOrDev is fine, but since you're replacing the canonical one, I'd just keep getPackageVersion(): string (always-returns-a-string semantics, with 'dev' baked in) so callers don't need to know the suffix. The behavior is now "the package version, or DEV_VERSION for unstamped checkouts."

So: yes, replace it. But fold the dev guard into the version-mismatch warner in the same PR, double-check the path depth, and split off the manifest-file deletion.

@benesjan benesjan May 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decided to go ahead and tackled this in #22941

}

return undefined;
}
Loading