fix(studio): inject version from package.json and include in all telemetry events#1151
Conversation
…metry events The Vite build relied on process.env.npm_package_version which is only set when invoked through npm/bun run scripts. CI builds running vite build directly got "dev" as the version. Read package.json directly so the version is always correct regardless of invocation method. Also add studio_version to the BrowserSystemMeta interface so the new telemetry system (studio_session_start, studio_render_start, etc.) includes the deployed version in every event.
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at 57d26a09. Small + correct. The build-context-dependence on process.env.npm_package_version is a real foot-gun in CI paths that don't go through bun run / npm run, and the package.json-direct read is the standard fix. One small cosmetic.
Pattern is established + consistent
__STUDIO_VERSION__ is already injected + consumed by packages/studio/src/utils/studioTelemetry.ts:51 with the exact same shape:
studio_version: typeof __STUDIO_VERSION__ !== "undefined" ? __STUDIO_VERSION__ : "dev",
declare const __STUDIO_VERSION__: string;This PR adds the same lines to the new telemetry system at system.ts:48,53 — symmetric, idempotent. Nothing surprising.
The Vite config switch from process.env.npm_package_version ?? "dev" to studioPkg.version is correct: npm_package_version is set ONLY when the build runs through npm/bun/pnpm script execution. Bare npx vite build (no script-runner wrapper) leaves it undefined, falling through to "dev". The package.json-direct read works in every invocation path. Build-time, not runtime — no leakage of other package.json fields into the JS bundle.
Imports are already there
import { readFileSync, readdirSync, existsSync, lstatSync, realpathSync } from "node:fs";
import { join, resolve } from "node:path";Both readFileSync and resolve are already imported at the top of the file (line 3-4). No new dependency. ✓
__dirname is already used at line 13 of the same file (resolve(__dirname, "../core/src/inline-scripts/hyperframe.ts")), so the CommonJS-style construct is available in this vite-config context.
Sister-bug check — clean
I grepped both directions:
__STUDIO_VERSION__users:studioTelemetry.ts:51(already wired) +system.ts:48(this PR). Both pick up the same Vite-injected constant. ✓npm_package_versionusers in studio: just the one invite.config.ts:170that this PR replaces. No lingering callsites. ✓
Cosmetic
declare const __STUDIO_VERSION__: string; at the bottom of the file (line 53), after it's used in getBrowserSystemMeta at line 48. TypeScript hoists declare at module scope so the type-check works regardless of position, but conventionally the declaration goes near the top of the file (right after imports) so a reader sees the ambient identifier before the consumer. studioTelemetry.ts:61 has the same bottom-of-file placement so it's a within-codebase convention; consistency wins over my mild preference. Leave it.
Dashboard correlation
Once this lands, studio_session_start and studio_render_start events emitted through the new system will carry studio_version with the actual built version (instead of "dev"). Combined with hf#1149's failed_stage, the observability dashboard gains two new filters: "which Studio version is generating which kinds of render errors at which pipeline stages." That's a meaningful unblock for the team's ability to attribute regressions to specific Studio releases.
Verdict
LGTM. Trivial diff, correct fix, established pattern, no surprises. Stamp held — James gates.
— Rames Jusso (hyperframes)
vanceingalls
left a comment
There was a problem hiding this comment.
Review
Clean, well-scoped fix. process.env.npm_package_version is only populated when invoked through npm/bun scripts, so bare vite build in CI was silently falling back to "dev". Reading package.json directly is the right solution. The typeof __STUDIO_VERSION__ !== "undefined" guard in system.ts is the correct pattern for Vite define tokens — esbuild special-cases typeof X so it resolves correctly in Vite builds and falls back safely in non-Vite consumers.
CI: base is main, all required checks green (full set including Windows render). ✓
Non-blocking
Missing test for getBrowserSystemMeta() output (should-fix): client.test.ts only exercises shouldTrack(), and events.test.ts mocks the client out. Neither verifies that studio_version actually appears in the metadata passed to PostHog. A happy-dom test that stubs __STUDIO_VERSION__ and asserts getBrowserSystemMeta().studio_version === "1.2.3" would close the gap — this is telemetry instrumentation, so the correctness guarantee matters.
EMPTY_META.studio_version hardcoded "dev" (nit): The SSR/no-DOM path returns EMPTY_META unconditionally. Since __STUDIO_VERSION__ is a build-time constant (not a window-access), it's available in the SSR path too — could use the same typeof guard for consistency. Practical impact is low since Studio telemetry rarely fires server-side.
Forward-looking question (not blocking): studioTelemetry.ts (legacy) and telemetry/client.ts (new) are two separate PostHog clients coexisting in the bundle. Is the legacy one on a deprecation path?
LGTM. Correct fix with the right pattern.
— Vai
| device_pixel_ratio: 0, | ||
| timezone_offset_minutes: 0, | ||
| is_mobile: false, | ||
| studio_version: "dev", |
There was a problem hiding this comment.
Nit: EMPTY_META.studio_version is hardcoded "dev", but __STUDIO_VERSION__ is a build-time constant (not a DOM access), so it's available in the SSR/no-DOM path too. Could use the same typeof __STUDIO_VERSION__ !== "undefined" guard here for consistency — though Studio telemetry rarely fires server-side so impact is low.
Summary
package.jsoninstead of relying onprocess.env.npm_package_version— this was falling back to"dev"in any build context that didn't go throughnpm runorbun run(e.g., barevite buildin CI)studio_versiontoBrowserSystemMetaso all events emitted through the new telemetry system (includingstudio_session_startandstudio_render_start) include the deployed version — previously only the legacytrackStudioEvent()system attached itTest plan
bun run buildinpackages/studio— verify__STUDIO_VERSION__is injected as the package.json version (not "dev")npx vite buildinpackages/studio— verify same resultstudio_session_startevents in PostHog includestudio_versionwith the actual version number