feat: add a warning when the package license changes#2188
feat: add a warning when the package license changes#2188aisiklar wants to merge 2 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
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
|
📝 WalkthroughWalkthroughThis pull request introduces license change detection across npm package versions. A new composable Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f7c1c88e-8b86-4447-9fdb-bfbc10c1ff70
📒 Files selected for processing (6)
app/components/LicenseDisplay.vueapp/composables/useLicenseChanges.tsapp/pages/package/[[org]]/[name].vuei18n/locales/en.jsoni18n/locales/tr-TR.jsoni18n/schema.json
| :title="$t('package.license.view_spdx')" | ||
| > | ||
| {{ token.value }} | ||
| {{ token.value }}!! |
There was a problem hiding this comment.
Remove the unintended !! suffix from linked licence labels.
This renders values like MIT!!, which is incorrect and user-visible noise.
Suggested fix
- {{ token.value }}!!
+ {{ token.value }}📝 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.
| {{ token.value }}!! | |
| {{ token.value }} |
| <div v-if="changes.length > 0" class="text-red-500 flex justify-start items-center gap-x-1"> | ||
| <p>change!</p> | ||
| <TooltipApp interactive position="top"> |
There was a problem hiding this comment.
Localise the change badge text instead of hard-coding English.
change! is currently not translated, so Turkish users still see English in this new UI element.
Suggested direction
- <p>change!</p>
+ <p>{{ $t('package.versions.license_change_badge') }}</p>Add package.versions.license_change_badge in locale files + schema.
| versions.sort((a, b) => { | ||
| const parse = (v: string) => v.split('.').map(Number) | ||
| const [aMajor, aMinor, aPatch] = parse(a.version as string) | ||
| const [bMajor, bMinor, bPatch] = parse(b.version as string) | ||
| if (aMajor !== bMajor) return aMajor! - bMajor! | ||
| if (aMinor !== bMinor) return aMinor! - bMinor! | ||
| return aPatch! - bPatch! |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In JavaScript, what does Number('0-beta') return, and how does Array.prototype.sort behave when the comparator returns NaN?
💡 Result:
Number('0-beta') returns NaN, because the Number constructor converts strings by parsing them as number literals, and parsing fails for invalid formats like '0-beta', resulting in NaN. Array.prototype.sort treats a comparator return value of NaN as equivalent to 0 (equal), per the ECMAScript specification: if the comparator returns NaN, it returns +0, meaning the elements are considered equal and their relative order is preserved (stable sort) or implementation-dependent if the comparator is inconsistent. Returning NaN from the comparator leads to undefined sort behavior across engines if not strictly equal comparisons.
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/Number
- 3: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort
🏁 Script executed:
# First, let's look at the file structure and find the file
fd -t f "useLicenseChanges.ts" --max-one-resultRepository: npmx-dev/npmx.dev
Length of output: 294
🏁 Script executed:
# Check the actual code at lines 52-58
cat -n app/composables/useLicenseChanges.ts | sed -n '40,70p'Repository: npmx-dev/npmx.dev
Length of output: 1465
🏁 Script executed:
# Search for NpmRegistryResponse interface definition
rg -A 10 "interface NpmRegistryResponse" app/composables/Repository: npmx-dev/npmx.dev
Length of output: 599
🏁 Script executed:
# Check if there's a broader pattern of version handling in the codebase
rg "\.split\('\.'\)\.map\(Number\)" --type tsRepository: npmx-dev/npmx.dev
Length of output: 880
🏁 Script executed:
# Look for the data source and how versions are fetched
rg -B 5 -A 15 "versions\.sort" app/composables/useLicenseChanges.tsRepository: npmx-dev/npmx.dev
Length of output: 911
🏁 Script executed:
# Find NpmRegistryVersion type definition
rg -B 2 -A 8 "type NpmRegistryVersion|interface NpmRegistryVersion" app/composables/Repository: npmx-dev/npmx.dev
Length of output: 758
🏁 Script executed:
# Check the full file to see all type definitions
cat -n app/composables/useLicenseChanges.ts | head -40Repository: npmx-dev/npmx.dev
Length of output: 1293
🏁 Script executed:
# Check if there's test data showing what npm registry actually returns
fd -t f "\.json$" test/fixtures/npm-registry/packuments/ | head -5Repository: npmx-dev/npmx.dev
Length of output: 337
🏁 Script executed:
# Look at a sample npm registry fixture to see if it has time field
cat test/fixtures/npm-registry/packuments/*/package.json 2>/dev/null | head -100Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
# Let's look at the structure of one fixture file
head -50 test/fixtures/npm-registry/packuments/@nuxt/kit.json | jq 'keys' 2>/dev/null || head -50 test/fixtures/npm-registry/packuments/@nuxt/kit.jsonRepository: npmx-dev/npmx.dev
Length of output: 1475
🏁 Script executed:
# Check if any other code accesses time field from npm registry
rg "\.time\[|\.time\?" app/ server/ --type tsRepository: npmx-dev/npmx.dev
Length of output: 1018
🏁 Script executed:
# Let's see a real npm registry response structure by checking what fields are in the fixture
python3 << 'EOF'
import json
try:
with open('test/fixtures/npm-registry/packuments/@nuxt/kit.json', 'r') as f:
data = json.load(f)
# Show top-level keys
print("Top-level keys:", list(data.keys())[:20])
if 'time' in data:
print("\ntime field exists:", type(data['time']))
print("Sample time entries:", list(data['time'].items())[:3])
if 'versions' in data:
print("\nVersions count:", len(data['versions']))
sample_version = next(iter(data['versions'].values()))
print("Sample version structure:", {k: type(v).__name__ for k, v in list(sample_version.items())[:5]})
except Exception as e:
print(f"Error: {e}")
EOFRepository: npmx-dev/npmx.dev
Length of output: 559
🏁 Script executed:
# Check how versions are actually ordered in the fixture
python3 << 'EOF'
import json
try:
with open('test/fixtures/npm-registry/packuments/@nuxt/kit.json', 'r') as f:
data = json.load(f)
versions = list(data['versions'].keys())
print(f"Total versions: {len(versions)}")
print(f"First 5: {versions[:5]}")
print(f"Last 5: {versions[-5:]}")
# Check for prerelease versions
prerelease = [v for v in versions if '-' in v]
print(f"Prerelease versions: {len(prerelease)}")
if prerelease:
print(f"Sample prereleases: {prerelease[:3]}")
except Exception as e:
print(f"Error: {e}")
EOFRepository: npmx-dev/npmx.dev
Length of output: 279
Add time field to NpmRegistryResponse interface and use chronological sorting by version release date.
The version sort comparator breaks with prerelease versions (e.g. 4.0.0-alpha.4 produces NaN when parsing 0-alpha). This causes unstable ordering, potentially reporting licence changes in the wrong sequence. The npm registry provides a time field mapping versions to ISO timestamps, which other parts of the codebase already use. Update the interface and sort logic to prioritise time-based ordering:
Suggested fix
interface NpmRegistryResponse {
versions: Record<
string,
{
version: string
license?: string
}
>
+ time?: Record<string, string>
}
versions.sort((a, b) => {
- const parse = (v: string) => v.split('.').map(Number)
- const [aMajor, aMinor, aPatch] = parse(a.version as string)
- const [bMajor, bMinor, bPatch] = parse(b.version as string)
- if (aMajor !== bMajor) return aMajor! - bMajor!
- if (aMinor !== bMinor) return aMinor! - bMinor!
- return aPatch! - bPatch!
+ const aTime = data.time?.[a.version]
+ const bTime = data.time?.[b.version]
+ if (aTime && bTime) {
+ return new Date(aTime).getTime() - new Date(bTime).getTime()
+ }
+ return a.version.localeCompare(b.version, undefined, {
+ numeric: true,
+ sensitivity: 'base',
+ })
})🧰 Tools
🪛 GitHub Check: 🤖 Autofix code
[warning] 53-53: eslint-plugin-unicorn(consistent-function-scoping)
Function parse does not capture any variables from its parent scope
🔗 Linked issue
fixes #1826
🧭 Context
added info/warning beneath the license info, "changed!" if there is a change in the license with additional tooltip which shows info about the change, similar to "from x to y at version a.b".
Also modified the Turkish language support too.
📚 Description
as mentioned in the issue, sometimes license changes and it is important to notify the potential user of the package. This pull request is done to make sure that if there is a change in the license the user will be able to see it and also will get info about the details of the change (like from x license to y license and at version a.b.c)
The following changes are done to the codebase:
modified the [name].vue file: added a new props to LicenseDisplay component.
new composable, useLicenseChanges is written.
In the LicenseDisplay component, changes were made to use this composable and add new element for displaying info about the changed license.
Additionally, en.json and tr-TR.json and schema.json files are modified, to add i18n infrastructure and add Turkish language support (for other languages, additional changes are required)
before:

after:
