Show warning in glue dev if old glue-runtime version is being used#30
Conversation
Greptile SummaryThis PR adds a warning in How it works:
Key things to verify:
Confidence Score: 3/5Mostly safe but has a correctness bug in the version comparison that can produce a misleading downgrade warning for users on newer/pre-release versions. The refactoring of
Important Files Changed
Reviews (1): Last reviewed commit: "use ink to render warning so it doesn't ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (latestVersion === currentVersion) { | ||
| return undefined; | ||
| } | ||
| return { currentVersion, latestVersion }; |
There was a problem hiding this comment.
The check latestVersion === currentVersion uses strict string equality, meaning getOutdatedStreakRuntimeVersion will return an OutdatedRuntimeInfo object whenever the two strings differ — including the case where the user's locked version is newer than the JSR-published latest (e.g., they previously locked a pre-release like 0.3.0-beta.1, or latest was temporarily rolled back on JSR). In that scenario the UI would show:
A newer @streak-glue/runtime version is available (0.3.0-beta.1 -> 0.2.35).
Update it with: deno update --latest @streak-glue/runtime
…which is both misleading and incorrect. A semantic version comparison (e.g., using the semver standard library) should be used so a warning is only shown when latestVersion is strictly greater than currentVersion.
Consider importing @std/semver and using greaterThan:
import { greaterThan, parse } from "@std/semver";
// ...
if (!greaterThan(parse(latestVersion), parse(currentVersion))) {
return undefined;
}![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a background check to notify users when the @streak-glue/runtime version is outdated, along with a new utility for Deno configuration discovery and a global verbose flag. Feedback recommends making the version check non-blocking to improve startup performance, guarding UI updates to prevent errors after the UI has unmounted, and utilizing safer parsing for the Deno lockfile to enhance robustness.
| const lifelineReconnectionEvents = pushableV<void>({ objectMode: true }); | ||
|
|
||
| const glueName = await getGlueName(filename, options.name); | ||
| await warnIfStreakRuntimeIsOutdated(filename, options.verbose); |
There was a problem hiding this comment.
Awaiting warnIfStreakRuntimeIsOutdated here blocks the startup of the dev session until the network request to JSR completes. This can cause the CLI to feel unresponsive for several seconds on slower connections. It is better to trigger this check in the background to allow the dev server to start immediately.
| await warnIfStreakRuntimeIsOutdated(filename, options.verbose); | |
| warnIfStreakRuntimeIsOutdated(filename, options.verbose); |
| async function warnIfStreakRuntimeIsOutdated(filename: string, verbose: boolean): Promise<void> { | ||
| try { | ||
| const outdatedRuntime = await getOutdatedStreakRuntimeVersion(filename); | ||
| if (!outdatedRuntime) { | ||
| return; | ||
| } | ||
| devProgressProps.outdatedRuntimeWarningInfo = outdatedRuntime; | ||
| renderUI(); | ||
| } catch (e) { | ||
| if (verbose) { | ||
| console.error("Caught error:", e); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
When running the version check in the background, we should ensure that renderUI() is only called if the UI is still active. This prevents a "zombie" UI from being rendered if the check completes after the setup phase has finished and the UI has been unmounted.
async function warnIfStreakRuntimeIsOutdated(filename: string, verbose: boolean): Promise<void> {
try {
const outdatedRuntime = await getOutdatedStreakRuntimeVersion(filename);
if (!outdatedRuntime) {
return;
}
devProgressProps.outdatedRuntimeWarningInfo = outdatedRuntime;
if (inkInstance) {
renderUI();
}
} catch (e) {
if (verbose) {
console.error("Caught error:", e);
}
}
}
1 participant
No description provided.