Skip to content

fix: don't diff install size with first version#2175

Merged
ghostdevv merged 4 commits intomainfrom
jg/big-diffs-in-early-days
Mar 21, 2026
Merged

fix: don't diff install size with first version#2175
ghostdevv merged 4 commits intomainfrom
jg/big-diffs-in-early-days

Conversation

@43081j
Copy link
Contributor

@43081j 43081j commented Mar 20, 2026

Most initial versions are empty or unfinished, so generally show up as a
large bump in file size to the 2nd version.

This is misleading so we should just skip those.

🔗 Linked issue

🧭 Context

📚 Description

Most initial versions are empty or unfinished, so generally show up as a
large bump in file size to the 2nd version.

This is misleading so we should just skip those.
@vercel
Copy link

vercel bot commented Mar 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Mar 21, 2026 4:21am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Mar 21, 2026 4:21am
npmx-lunaria Ignored Ignored Mar 21, 2026 4:21am

Request Review

@ghostdevv ghostdevv enabled auto-merge March 20, 2026 21:48
@ghostdevv ghostdevv disabled auto-merge March 20, 2026 21:49
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6a4f74cf-f91c-4138-9827-533eed633579

📥 Commits

Reviewing files that changed from the base of the PR and between df7d8c2 and 6f34fd2.

📒 Files selected for processing (1)
  • test/nuxt/composables/use-install-size-diff.spec.ts

📝 Walkthrough

Walkthrough

The change modifies the getComparisonVersion function in app/composables/useInstallSizeDiff.ts to enforce a stricter baseline requirement for installation size comparisons. The guard condition was updated from currentIdx <= 0 to currentIdx <= 1, which ensures the function returns null when the resolved version is either the first or second stable entry in the list. This prevents comparisons when there is insufficient historical data. An explanatory inline comment was added to document this behaviour. The change affects only internal logic with no impact on exported or public entity declarations.

Possibly related PRs

Suggested reviewers

  • danielroe
  • trueberryless
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The PR description directly addresses the changeset, explaining why the first version should be skipped for install-size comparisons.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jg/big-diffs-in-early-days

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.

OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
app/composables/useInstallSizeDiff.ts (1)

38-38: Consider removing the non-null assertion for strict type safety.

The non-null assertion operator bypasses TypeScript's type checking. Whilst the guard ensures the array access is logically safe, a defensive check would better align with the strict type-safety guideline.

🛡️ Proposed fix to add explicit bounds checking
- return stableVersions[currentIdx - 1]!
+ const prevVersion = stableVersions[currentIdx - 1]
+ if (!prevVersion) return null
+ return prevVersion

As per coding guidelines: "Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index"


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b223cee3-93cd-450d-9784-a20ca2248579

📥 Commits

Reviewing files that changed from the base of the PR and between 1cd901f and df7d8c2.

📒 Files selected for processing (1)
  • app/composables/useInstallSizeDiff.ts

@43081j 43081j marked this pull request as ready for review March 21, 2026 04:17
@ghostdevv ghostdevv marked this pull request as draft March 21, 2026 04:18
@43081j 43081j marked this pull request as ready for review March 21, 2026 04:19
@codecov
Copy link

codecov bot commented Mar 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@ghostdevv ghostdevv added this pull request to the merge queue Mar 21, 2026
Merged via the queue into main with commit 291c22d Mar 21, 2026
23 checks passed
@ghostdevv ghostdevv deleted the jg/big-diffs-in-early-days branch March 21, 2026 04:28
@github-actions github-actions bot mentioned this pull request Mar 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants