-
-
Notifications
You must be signed in to change notification settings - Fork 360
feat: add a warning when the package license changes #2188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,34 @@ | ||
| <script setup lang="ts"> | ||
| import { parseLicenseExpression } from '#shared/utils/spdx' | ||
|
|
||
| import { useLicenseChanges } from '~/composables/useLicenseChanges' | ||
| import { useI18n } from 'vue-i18n' | ||
|
|
||
| const props = defineProps<{ | ||
| license: string | ||
| packageName?: string | ||
| }>() | ||
|
|
||
| const { t } = useI18n() | ||
|
|
||
| const tokens = computed(() => parseLicenseExpression(props.license)) | ||
| const licenseChanges = useLicenseChanges(() => props.packageName) | ||
|
|
||
| const changes = computed(() => licenseChanges.data.value?.changes ?? []) | ||
|
|
||
| const licenseChangeText = computed(() => | ||
| changes.value | ||
| .map(item => | ||
| t('package.versions.license_change_item', { | ||
| from: item.from, | ||
| to: item.to, | ||
| version: item.version, | ||
| }), | ||
| ) | ||
| .join('; '), | ||
| ) | ||
|
|
||
| const hasAnyValidLicense = computed(() => tokens.value.some(t => t.type === 'license' && t.url)) | ||
| </script> | ||
|
|
||
| <template> | ||
|
|
@@ -21,7 +42,7 @@ | |
| class="link-subtle" | ||
| :title="$t('package.license.view_spdx')" | ||
| > | ||
| {{ token.value }} | ||
| {{ token.value }}!! | ||
| </a> | ||
| <span v-else-if="token.type === 'license'">{{ token.value }}</span> | ||
| <span v-else-if="token.type === 'operator'" class="text-4xs">{{ token.value }}</span> | ||
|
|
@@ -32,4 +53,26 @@ | |
| aria-hidden="true" | ||
| /> | ||
| </span> | ||
| <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"> | ||
|
Comment on lines
+56
to
+58
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Localise the change badge text instead of hard-coding English.
Suggested direction- <p>change!</p>
+ <p>{{ $t('package.versions.license_change_badge') }}</p>Add |
||
| <span | ||
| tabindex="0" | ||
| class="block cursor-help shrink-0 -m-2 p-2 -me-1 focus-visible:outline-2 focus-visible:outline-accent/70 rounded" | ||
| > | ||
| <span | ||
| class="block i-lucide:info w-3.5 h-3.5 text-fg-subtle" | ||
| role="img" | ||
| :aria-label="$t('package.versions.license_change_help')" | ||
| /> | ||
| </span> | ||
| <template #content> | ||
| <p class="text-xs text-fg-muted"> | ||
| <i18n-t keypath="package.versions.changed_license" tag="span"> | ||
| <template #license_change>{{ licenseChangeText }}</template> | ||
| </i18n-t> | ||
| </p> | ||
| </template> | ||
| </TooltipApp> | ||
| </div> | ||
| </template> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| import type { MaybeRefOrGetter } from 'vue' | ||
| import { toValue } from 'vue' | ||
|
|
||
| export interface LicenseChange { | ||
| from: string | ||
| to: string | ||
| version: string | ||
| } | ||
|
|
||
| export interface LicenseChangesResult { | ||
| changes: LicenseChange[] | ||
| } | ||
|
|
||
| // Type definitions for npm registry response | ||
| interface NpmRegistryVersion { | ||
| version: string | ||
| license?: string | ||
| } | ||
|
|
||
| // for registry responses of $fetch function, the type includes the key versions as well as many others too. | ||
| interface NpmRegistryResponse { | ||
| versions: Record< | ||
| string, | ||
| { | ||
| version: string | ||
| license?: string | ||
| } | ||
| > | ||
| } | ||
|
|
||
| /** | ||
| * Composable to detect license changes across all versions of a package | ||
| */ | ||
| export function useLicenseChanges(packageName: MaybeRefOrGetter<string | null | undefined>) { | ||
| return useAsyncData<LicenseChangesResult>( | ||
| () => `license-changes:${toValue(packageName)}`, | ||
| async () => { | ||
| const name = toValue(packageName) | ||
| if (!name) return { changes: [] } | ||
|
|
||
| // Fetch full package metadata from npm registry | ||
| const url = `https://registry.npmjs.org/${name}` | ||
| const data = await $fetch<NpmRegistryResponse>(url) | ||
|
|
||
| const changes: LicenseChange[] = [] | ||
| let prevLicense: string | undefined = undefined | ||
|
|
||
| // `data.versions` is an object with version keys | ||
| const versions = Object.values(data.versions) as NpmRegistryVersion[] | ||
|
|
||
| // Sort versions ascending to compare chronologically | ||
| 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! | ||
|
Comment on lines
+52
to
+58
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 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:
🏁 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. Suggested fixinterface 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) |
||
| }) | ||
|
|
||
| // Detect license changes | ||
| for (const version of versions) { | ||
| const license = (version.license as string) ?? 'UNKNOWN' | ||
| if (prevLicense && license !== prevLicense) { | ||
| changes.push({ | ||
| from: prevLicense, | ||
| to: license, | ||
| version: version.version as string, | ||
| }) | ||
| } | ||
| prevLicense = license | ||
| } | ||
| return { changes } | ||
| }, | ||
| { | ||
| default: () => ({ changes: [] }), | ||
| watch: [() => toValue(packageName)], | ||
| }, | ||
| ) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the unintended
!!suffix from linked licence labels.This renders values like
MIT!!, which is incorrect and user-visible noise.Suggested fix
📝 Committable suggestion