Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR removes NPMS score integration and all score-related features: deletes NPMS mock routing and helpers, removes badge strategies for quality/popularity/maintenance/score, strips the Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
i think the e2e badge tests need updating 👀 |
|
Hmm, I discovered a lot more places where "popularity", "quality", "maintainence" and "score" are used, I'll push a commit that removes those as well, but they might not be neccessary to remove 🤔 |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
|
Okay, I might have missed many use cases of the four scores throughout the project. I am not totally sure if they all use npms, but they seems to show up everywhere. I'm not sure if we should maybe just remove them from custom badges, if the other occurances even use npms or what exactly to do here 🤔 Also I think a lot of i18n files would still need to be updated (remove the translations), but I didn't do it for now, because I wanted to get confirmation, whether I am doing the right thing (we are speaking of removing about 1000 lines 😅). We can always revert the last commit, but I'd like to get a little guidance here... |
|
if you can fix the type errors, looks good to me 👍 edit: ah and some conflicts with main |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
i18n/locales/zh-TW.json (1)
887-890:⚠️ Potential issue | 🔴 CriticalRemove these orphaned sort option translations.
The
quality,popularity,maintenance, andscorekeys are no longer referenced in the codebase. TheSortKeytype definition inshared/types/preferences.tsonly includesdownloads-week,downloads-day,downloads-month,downloads-year, andupdated, and thePROVIDER_SORT_KEYSdefinition confirms these providers support onlyrelevance,downloads-week,updated, andname. These four translation entries are dead code and should be removed as part of this PR's cleanup.i18n/locales/ru-RU.json (1)
573-579:⚠️ Potential issue | 🟡 MinorNest
downloadunderpackage.sizeto match the intended key path.At Line 578,
downloadis currently created aspackage.downloadinstead ofpackage.size.download, which can leave the expected nested key unresolved for this locale.💡 Proposed fix
"size": { "b": "{size} байт", "kb": "{size} КБ", - "mb": "{size} МБ" - }, - "download": {} + "mb": "{size} МБ", + "download": {} + }i18n/locales/de-DE.json (1)
856-859:⚠️ Potential issue | 🟡 MinorRemove orphaned sort key translations from German locale.
Lines 856–859 contain translations for
quality,popularity,maintenance, andscoresort keys. These keys have been removed from the functional codebase—theSORT_KEYSconfiguration inshared/types/preferences.tsno longer includes them, and the code comment confirms "Neither provider returns useful quality/popularity/maintenance/score values". Remove these translations from the German locale to maintain consistency with the code changes.i18n/locales/bn-IN.json (1)
639-642:⚠️ Potential issue | 🟠 MajorRemove these non-functional sort option translations.
The
quality,popularity,maintenance, andscoresort keys at lines 639–642 are orphaned. They are not included inVALID_SORT_KEYSinshared/types/preferences.ts(which permits onlyrelevance,downloads-week,downloads-day,downloads-month,downloads-year,updated, andname). The codebase explicitly documents that these metrics "return useful values" from neither search provider—npm returns synthetic1values, and Algolia returns zeros. Any attempt to use these sort keys will fail validation and fall back todownloads-week-desc. Remove these orphaned translations from the i18n file.i18n/locales/cs-CZ.json (1)
868-871:⚠️ Potential issue | 🟠 MajorRemove unused sort option translations from all locale files.
The keys
quality,popularity,maintenance, andscoreunderfilters.sortare unused legacy entries from the npms provider. The codebase documents inshared/types/preferences.tsthat these return useless or synthetic values and explicitly excludes them fromPROVIDER_SORT_KEYS. The actual sorting implementation inapp/components/Package/ListToolbar.vuereferences only:relevance,downloads-week,downloads-day,downloads-month,downloads-year,published, andname.These unused keys remain across all 26+ locale files (including en.json, de-DE.json, ar.json, etc.) and should be removed as part of the npms cleanup to avoid maintaining dead translation strings.
i18n/locales/te-IN.json (1)
640-643:⚠️ Potential issue | 🟠 MajorRemove orphaned NPMS sort labels from translations.
Lines 640–643 contain translation entries for
quality,popularity,maintenance, andscoreunderfilters.sort. These sort options are not defined in the activeSortKeytype (shared/types/preferences.ts), which includes only:relevance,downloads-week,downloads-day,downloads-month,downloads-year,updated, andname. These four entries are unreferenced orphaned translations from the NPMS removal and should be deleted from this file and all other locale files.i18n/locales/hi-IN.json (1)
640-643:⚠️ Potential issue | 🟡 MinorDrop the leftover score-sort labels.
These are still the old NPMS-derived sort names. The corresponding score-based sort surface has been removed elsewhere in this PR, so keeping these translations behind leaves dead NPMS copy in
hi-IN.json.✂️ Proposed cleanup
"downloads_month": "डाउनलोड्स/महीना", "downloads_year": "डाउनलोड्स/वर्ष", - "name": "नाम", - "quality": "गुणवत्ता", - "popularity": "लोकप्रियता", - "maintenance": "रखरखाव", - "score": "स्कोर" + "name": "नाम"
🧹 Nitpick comments (3)
i18n/locales/pl-PL.json (1)
870-873: Consider removing orphaned sort keys once npms removal is confirmed.The
quality,popularity,maintenance, andscoresort translation keys at lines 870–873 may now be unused if the corresponding sort functionality has been removed with npms. Per the PR discussion, i18n removals are pending confirmation — this is a reminder to clean these up once the scope is finalised.i18n/locales/pt-BR.json (1)
1180-1180: Prefer omitting untranslated keys instead of adding empty objects.Using
{}placeholders for new message paths can produce empty sections or type mismatches at runtime, depending on how the key is read. It’s safer to omit these keys until translated and rely on fallback locale resolution.Suggested cleanup
- "version_invalid_url_format": {}, @@ - "translation_status": { - "table": {} - },Based on learnings: In this repository, non-English locale files are allowed to omit new or changed keys and translations are completed separately.
Also applies to: 1347-1349
i18n/locales/fr-FR.json (1)
599-599: Prefer omitting untranslated placeholder objects.Line 599 and Lines 1336-1339 add empty objects only. In this repo, untranslated non-English keys can be omitted, so removing these placeholders is usually cleaner until real French values are added.
Suggested cleanup
- "size": { - "b": "{size} o", - "kb": "{size} ko", - "mb": "{size} Mo" - }, - "download": {} + "size": { + "b": "{size} o", + "kb": "{size} ko", + "mb": "{size} Mo" + }- }, - "translation_status": { - "table": {} - }, - "action_bar": {} + }Based on learnings: In the npmx.dev project, new or changed entries in i18n translation files may be omitted from non-English languages, and missing non-English keys are acceptable.
Also applies to: 1336-1339
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01f019a5-8f9d-4a38-bc10-9290a579b2c4
📒 Files selected for processing (32)
app/components/Package/TableRow.vueapp/pages/search.vuedocs/content/2.guide/6.badges.mdi18n/locales/ar.jsoni18n/locales/az-AZ.jsoni18n/locales/bg-BG.jsoni18n/locales/bn-IN.jsoni18n/locales/cs-CZ.jsoni18n/locales/de-DE.jsoni18n/locales/en.jsoni18n/locales/es.jsoni18n/locales/fr-FR.jsoni18n/locales/hi-IN.jsoni18n/locales/hu-HU.jsoni18n/locales/id-ID.jsoni18n/locales/it-IT.jsoni18n/locales/ja-JP.jsoni18n/locales/kn-IN.jsoni18n/locales/nb-NO.jsoni18n/locales/ne-NP.jsoni18n/locales/pl-PL.jsoni18n/locales/pt-BR.jsoni18n/locales/ru-RU.jsoni18n/locales/ta-IN.jsoni18n/locales/te-IN.jsoni18n/locales/tr-TR.jsoni18n/locales/uk-UA.jsoni18n/locales/zh-CN.jsoni18n/locales/zh-TW.jsoni18n/schema.jsonserver/api/registry/badge/[type]/[...pkg].get.tstest/nuxt/a11y.spec.ts
💤 Files with no reviewable changes (7)
- app/pages/search.vue
- docs/content/2.guide/6.badges.md
- app/components/Package/TableRow.vue
- i18n/schema.json
- server/api/registry/badge/[type]/[...pkg].get.ts
- test/nuxt/a11y.spec.ts
- i18n/locales/en.json
|
Autofix seems to have run some i18n script which adds in empty translations. Feel free to commit here, I have to run 👋 |
43081j
force-pushed
the
refactor/remove-npms
branch
from
9654681 to
826c1d9
Compare
|
no clue what autofix is doing, why it adds all these empty keys... but if ci passes, whatever. ill believe whatever i18n lint we run is doing its job 😅 |
|
maybe this fixes it, let's see: 82d00cd |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
i18n/locales/uk-UA.json (1)
591-596:⚠️ Potential issue | 🟠 Major
downloadis at the wrong path and wrong value type.Line 596 adds
"download": {}as a sibling ofsize, and as an object leaf. This is likely to fail i18n lookup/type expectations for a text label.Proposed fix
"size": { "b": "{size} Б", "kb": "{size} кБ", - "mb": "{size} МБ" - }, - "download": {} + "mb": "{size} МБ", + "download": "Завантаження" + }
♻️ Duplicate comments (1)
i18n/locales/es.json (1)
594-595:⚠️ Potential issue | 🟡 MinorRe-nest
downloadunderpackage.size.Line 595 currently creates
package.download, notpackage.size.download. That breaks lookups expecting thesizenamespace.Suggested fix
"size": { "b": "{size} B", "kb": "{size} kB", - "mb": "{size} MB" - }, - "download": {} + "mb": "{size} MB", + "download": {} + }
🧹 Nitpick comments (1)
i18n/locales/uk-UA.json (1)
1334-1337: Avoid adding empty namespaces in a country-variant locale file.For
uk-UA, empty objects liketranslation_status.tableandaction_baradd noise and may create merge ambiguity. Prefer omitting them until there are actual variant-specific overrides.Based on learnings: In the npmx.dev project, for country locale variants, only keys that differ from the base language should be defined in the variant JSON file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 40920401-c3e5-4d79-88d3-414bfe2f9617
📒 Files selected for processing (27)
i18n/locales/ar.jsoni18n/locales/az-AZ.jsoni18n/locales/bg-BG.jsoni18n/locales/bn-IN.jsoni18n/locales/cs-CZ.jsoni18n/locales/de-DE.jsoni18n/locales/en.jsoni18n/locales/es.jsoni18n/locales/fr-FR.jsoni18n/locales/hi-IN.jsoni18n/locales/hu-HU.jsoni18n/locales/id-ID.jsoni18n/locales/it-IT.jsoni18n/locales/ja-JP.jsoni18n/locales/kn-IN.jsoni18n/locales/nb-NO.jsoni18n/locales/ne-NP.jsoni18n/locales/pl-PL.jsoni18n/locales/pt-BR.jsoni18n/locales/ru-RU.jsoni18n/locales/ta-IN.jsoni18n/locales/te-IN.jsoni18n/locales/tr-TR.jsoni18n/locales/uk-UA.jsoni18n/locales/zh-CN.jsoni18n/locales/zh-TW.jsoni18n/schema.json
💤 Files with no reviewable changes (1)
- i18n/schema.json
✅ Files skipped from review due to trivial changes (2)
- i18n/locales/az-AZ.json
- i18n/locales/ja-JP.json
🚧 Files skipped from review as they are similar to previous changes (16)
- i18n/locales/hu-HU.json
- i18n/locales/pt-BR.json
- i18n/locales/fr-FR.json
- i18n/locales/en.json
- i18n/locales/te-IN.json
- i18n/locales/ar.json
- i18n/locales/pl-PL.json
- i18n/locales/cs-CZ.json
- i18n/locales/zh-CN.json
- i18n/locales/ne-NP.json
- i18n/locales/tr-TR.json
- i18n/locales/bg-BG.json
- i18n/locales/nb-NO.json
- i18n/locales/hi-IN.json
- i18n/locales/zh-TW.json
- i18n/locales/ta-IN.json
| "file_size_warning": "{size} перевищує ліміт 250 КБ для порівняння", | ||
| "compare_versions": "зміни", | ||
| "compare_versions_title": "Порівняти з останньою версією", | ||
| "version_invalid_url_format": {}, |
There was a problem hiding this comment.
compare.version_invalid_url_format must not be an empty object.
Line 1167 defines a leaf translation key as {}. This should be a string message; otherwise, the validation message can render incorrectly or fail lookup.
Proposed fix
- "version_invalid_url_format": {},
+ "version_invalid_url_format": "Недійсний формат URL-адреси версії",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "version_invalid_url_format": {}, | |
| "version_invalid_url_format": "Недійсний формат URL-адреси версії", |
|
I hope we don't remove any feature that was valueable to many people: Just the be clear, I did not exactly investigate what I have removed here, I just searched for all occurances of the four percentage stats (quality, popularity, maintenance and overall score) 🪭 But I really agree, that the dependency of npms shouldn't exist and if we find a better alternative, we can implement the features again 👍 |
2 participants
🧭 Context
As discussed on Discord, we decided to remove all npms traces since the data is 4 years outdated.
📚 Description
https://npms.io uses and provides outdated data, like e.g. this Astro package, which is 4 years old:
The only place where npms is really used is custom badges, which provide SVGs via an API mainly for markdown in Readmes. We decided that no data is better than wrong data, therefore deleting all traces.