Skip to content

fix: show correct dependencies count - #787

Merged
danielroe merged 23 commits into
npmx-dev:mainfrom
gameroman:fix-compare-deps
Feb 3, 2026
Merged

fix: show correct dependencies count#787
danielroe merged 23 commits into
npmx-dev:mainfrom
gameroman:fix-compare-deps

Conversation

@gameroman

@gameromangameroman commented Feb 2, 2026

Copy link
Copy Markdown
Member

fixes#857
closes#868

@vercel

vercelBot commented Feb 2, 2026

Copy link
Copy Markdown
Contributor

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

ProjectDeploymentActionsUpdated (UTC)
npmx.devReadyReadyPreview, CommentFeb 3, 2026 10:25pm
2 Skipped Deployments
ProjectDeploymentActionsUpdated (UTC)
docs.npmx.devIgnoredIgnoredPreviewFeb 3, 2026 10:25pm
npmx-lunariaIgnoredIgnoredFeb 3, 2026 10:25pm

Request Review

@danielroe

Copy link
Copy Markdown
Member

would you update the tests? 🙏

@github-actions

github-actionsBot commented Feb 3, 2026

Copy link
Copy Markdown

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

FileNote
lunaria/files/cs-CZ.jsonLocalization changed, will be marked as complete. 🔄️
lunaria/files/en-GB.jsonLocalization added, will be marked as complete. 🔄️
lunaria/files/en-US.jsonSource changed, localizations will be marked as outdated.
lunaria/files/es-419.jsonLocalization changed, will be marked as complete. 🔄️
lunaria/files/es-ES.jsonLocalization changed, will be marked as complete. 🔄️
lunaria/files/fr-FR.jsonLocalization changed, will be marked as complete. 🔄️
lunaria/files/mr-IN.jsonLocalization added, will be marked as complete. 🔄️
lunaria/files/pl-PL.jsonLocalization changed, will be marked as complete. 🔄️
lunaria/files/zh-CN.jsonLocalization changed, will be marked as complete. 🔄️
Warnings reference
IconDescription
🔄️The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

@coderabbitai

coderabbitaiBot commented Feb 3, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@gameroman has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 4 minutes and 12 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

A new numeric field directDeps: number | null was added to package comparison data and is populated using a new utility getDependencyCount in app/utils/npm/dependency-count.ts. The dependencies facet now reads from directDeps (warning threshold changed), while installSize.dependencyCount remains available for the totalDependencies facet. totalDependencies is no longer marked as coming soon and is included by default. Tests and UI facet selection were updated to remove special-casing for a coming‑soon totalDependencies facet.

Possibly related PRs

Suggested reviewers

  • danielroe
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description check✅ PassedThe PR description references issue #857 and PR #868, which are directly related to the changeset that fixes the direct dependencies count display.
Linked Issues check✅ PassedThe PR successfully implements the primary objective from issue #857: fixing the direct deps facet to show only direct dependencies instead of total dependencies, and enables the total deps feature previously marked as coming soon.
Out of Scope Changes check✅ PassedAll changes are directly aligned with issue #857 objectives: introducing direct dependency tracking, restructuring facet logic, creating a reusable utility function, and updating tests to reflect the removal of the coming-soon flag.

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

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

@coderabbitaicoderabbitaiBot left a comment

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.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
app/composables/usePackageComparison.ts (2)

370-397: ⚠️ Potential issue | 🟠 Major

Wrap switch cases in blocks and correctly handle zero dependencies.

The case statements lack blocks, triggering Biome's noSwitchDeclarations rule. Additionally, if (!data.directDeps) incorrectly filters out valid 0 values; use if (data.directDeps == null) to check only for null/undefined. Wrap 'dependencies', 'deprecated', and 'totalDependencies' cases in blocks.

🧩 Proposed fix
- case 'dependencies':- if (!data.directDeps) return null- const depCount = data.directDeps- return {- raw: depCount,- display: String(depCount),- status: depCount > 10 ? 'warning' : 'neutral',- }+ case 'dependencies': {+ if (data.directDeps == null) return null+ const depCount = data.directDeps+ return {+ raw: depCount,+ display: String(depCount),+ status: depCount > 10 ? 'warning' : 'neutral',+ }+ }- case 'deprecated':- const isDeprecated = !!data.metadata?.deprecated- return {- raw: isDeprecated,- display: isDeprecated- ? t('compare.facets.values.deprecated')- : t('compare.facets.values.not_deprecated'),- status: isDeprecated ? 'bad' : 'good',- }+ case 'deprecated': {+ const isDeprecated = !!data.metadata?.deprecated+ return {+ raw: isDeprecated,+ display: isDeprecated+ ? t('compare.facets.values.deprecated')+ : t('compare.facets.values.not_deprecated'),+ status: isDeprecated ? 'bad' : 'good',+ }+ }- case 'totalDependencies':- if (!data.installSize) return null- const totalDepCount = data.installSize.dependencyCount- return {- raw: totalDepCount,- display: String(totalDepCount),- status: totalDepCount > 50 ? 'warning' : 'neutral',- }+ case 'totalDependencies': {+ if (!data.installSize) return null+ const totalDepCount = data.installSize.dependencyCount+ return {+ raw: totalDepCount,+ display: String(totalDepCount),+ status: totalDepCount > 50 ? 'warning' : 'neutral',+ }+ }

116-150: ⚠️ Potential issue | 🟠 Major

Remove the usePackage call; compute direct deps from the already-fetched versionData.

Calling usePackage inside an async callback violates Nuxt 3's requirement that composables be called synchronously in setup context. As a result, pkg.data.value is undefined here, collapsing requestedVersion to undefined, and you lose the dependency count. You already have the version data from the registry response, so pass versionData directly to getDependencyCount instead.

Proposed fix
- const pkg = usePackage(name, latestVersion)- const requestedVersion = pkg.data.value?.requestedVersion-- const versionData = pkgData.versions[latestVersion]+ const versionData = pkgData.versions[latestVersion]
const packageSize = versionData?.dist?.unpackedSize
@@
- directDeps: getDependencyCount(requestedVersion ?? null),+ directDeps: getDependencyCount(versionData ?? null),

@gameroman

Copy link
Copy Markdown
MemberAuthor

Thank you for coderabbit

@gameroman

Copy link
Copy Markdown
MemberAuthor

I think it is ready now

Comment threadtest/nuxt/composables/use-facet-selection.spec.ts
@danielroe

Copy link
Copy Markdown
Member

would you also see whether there are any improvements to include from #868 ? I plan to merge this PR with a credit to @shamilkotta as they missed that you had already submitted this...

@gameroman

Copy link
Copy Markdown
MemberAuthor

I was thinking if comingSoon could be removed completely or if it is still needed?

@gameroman

Copy link
Copy Markdown
MemberAuthor

would you also see whether there are any improvements to include from #868?

Yes, will check and apply any improvements from that PR

@coderabbitaicoderabbitaiBot left a comment

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.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/composables/usePackageComparison.ts (1)

235-240: ⚠️ Potential issue | 🟡 Minor

Fix loading state for dependency facets.
dependencies now uses fast directDeps, but isFacetLoading still treats it as slow and doesn’t include totalDependencies, which is slow. This will show a misleading loading state.

🔧 Proposed fix
- return facet === 'installSize' || facet === 'dependencies'+ return facet === 'installSize' || facet === 'totalDependencies'

Also applies to: 368-395

@codecov

codecovBot commented Feb 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.00000% with 21 lines in your changes missing coverage. Please review.

Files with missing linesPatch %Lines
app/composables/usePackageComparison.ts9.09%17 Missing and 3 partials ⚠️
app/utils/npm/dependency-count.ts66.66%1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@danielroe
danielroe merged commit 3226764 into npmx-dev:mainFeb 3, 2026
15 of 16 checks passed
@serhalp

Copy link
Copy Markdown
Member

awesome, thank you for fixing this! 🙌🏼

Sign up for freeto 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.

package compare table "direct deps" facet actually shows total deps

3 participants

@gameroman@danielroe@serhalp