fix: get url from pkg.repository when string - #3173
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughRepository metadata now accepts string and object forms. URL normalisation, package image rendering, registry association, and changelog detection handle both forms. Repository comparisons use ChangesRepository URL handling
Possibly related PRs
Mergeability Score: 🟡 Moderate · up to The repository URL fallback can accept an invalid empty metadata shape because null is not excluded before the type transformation, weakening compile-time protection for package links. Merge should wait for this type to exclude null or for explicit owner acceptance. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/utils/changelog/detectChangelog.ts (1)
23-33: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPass the parsed directory to both checks.
parseRepositoryInfoalready normalisesrepository.directoryintorepoRef.directory. Both calls readpkg.repository?.directoryagain. PassrepoRef.directoryto keep normalisation in one place and support string-valued repositories without raw object property access.Suggested change
- const [releases, releasesError] = await checkReleases(repoRef, pkg.repository?.directory) + const [releases, releasesError] = await checkReleases(repoRef, repoRef.directory) ... - const changelog = await checkChangelogFile(repoRef, pkg.repository?.directory) + const changelog = await checkChangelogFile(repoRef, repoRef.directory)As per coding guidelines, “Ensure you write strictly type-safe code”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/utils/changelog/detectChangelog.ts` around lines 23 - 33, Update both checkReleases and checkChangelogFile calls in detectChangelog to pass repoRef.directory instead of pkg.repository?.directory, preserving the normalized directory from parseRepositoryInfo and ensuring type-safe handling of string-valued repositories.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/api/registry/analysis/`[...pkg].get.ts:
- Around line 150-170: Update the repository type definitions for
PackageWithMeta and ExtendedPackageJson to allow either repository metadata
objects or bare strings, then adjust detectChangelog.ts to narrow string
repository values before accessing the directory property. Preserve the existing
object-based directory handling.
---
Nitpick comments:
In `@server/utils/changelog/detectChangelog.ts`:
- Around line 23-33: Update both checkReleases and checkChangelogFile calls in
detectChangelog to pass repoRef.directory instead of pkg.repository?.directory,
preserving the normalized directory from parseRepositoryInfo and ensuring
type-safe handling of string-valued repositories.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3953b5ca-1399-446e-aa70-f52a3ff3a605
📒 Files selected for processing (4)
app/composables/useRepositoryUrl.tsserver/api/registry/analysis/[...pkg].get.tsserver/utils/changelog/detectChangelog.tstest/nuxt/composables/use-repository-url.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shared/types/npm-registry.ts`:
- Line 87: Update SlimPackumentVersion to override its inherited repository
field with the same object-or-string type used by the package-level repository
definition, preserving existing optional fields. Add a type-level test that
accepts a string repository on a SlimPackumentVersion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4953aa4f-0664-4205-a58a-d22a9706b20e
📒 Files selected for processing (5)
app/components/OgImage/Package.takumi.vueserver/utils/changelog/detectChangelog.tsshared/types/npm-registry.tsshared/utils/package-analysis.tstest/nuxt/composables/use-repository-url.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- test/nuxt/composables/use-repository-url.spec.ts
- server/utils/changelog/detectChangelog.ts
WilcoSp
changed the title
| import { describe, expect, it } from 'vitest' | ||
|
|
||
| type RequestedVersion = Exclude<SlimPackument['requestedVersion'], null> | ||
| type RequestedVersion = Omit<Exclude<SlimPackument['requestedVersion'], null>, 'repository'> & { |
There was a problem hiding this comment.
Why not import the type from app/composables/useRepositoryUrl.ts?
There was a problem hiding this comment.
I still need to figure things out as it still seems to be that Omit still drops everything in a type instead of only "repository"
but when things with omit are solved, yes it would be possible
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/composables/useRepositoryUrl.ts`:
- Around line 4-6: Update the RequestedVersion type alias to exclude null from
SlimPackument['requestedVersion'] before applying Omit, using the existing
repository override and null union so the resulting type does not permit an
empty object from omitting fields on null.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 70a05311-85c6-4002-b198-c18201fadbca
📒 Files selected for processing (2)
app/composables/useRepositoryUrl.tstest/nuxt/composables/use-repository-url.spec.ts
| export type RequestedVersion = | ||
| | (Omit<SlimPackument['requestedVersion'], 'repository'> & { repository?: string | Repository }) | ||
| | null |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tsc_bin="${TSC_BIN:-tsc}"
command -v "$tsc_bin" >/dev/null
probe="$(mktemp --suffix=.ts)"
trap 'rm -f "$probe"' EXIT
cat >"$probe" <<'EOF'
type Version = { version: string; repository?: { url?: string } } | null
type Broken = Omit<Version, 'repository'>
type Fixed = Omit<Exclude<Version, null>, 'repository'>
declare const broken: Broken
// `@ts-expect-error`: the nullable Omit form must not retain `version`.
const brokenVersion: string = broken.version
declare const fixed: Fixed
const fixedVersion: string = fixed.version
void brokenVersion
void fixedVersion
EOF
"$tsc_bin" --strict --noEmit "$probe"
rg -n 'Omit<.*SlimPackument.*requestedVersion|Omit<Exclude<.*SlimPackument.*requestedVersion' \
app/composables/useRepositoryUrl.ts \
test/nuxt/composables/use-repository-url.spec.tsRepository: npmx-dev/npmx.dev
Length of output: 297
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tsc_bin="${TSC_BIN:-tsc}"
command -v "$tsc_bin" >/dev/null
probe="$(mktemp --suffix=.ts)"
trap 'rm -f "$probe"' EXIT
cat >"$probe" <<'EOF'
type Version = { version: string; repository?: { url?: string } } | null
type Broken = Omit<Version, 'repository'>
type Fixed = Omit<Exclude<Version, null>, 'repository'>
declare const broken: Broken
// `@ts-expect-error`: the nullable Omit form must not retain `version`.
const brokenVersion: string = broken.version
declare const fixed: Fixed
const fixedVersion: string = fixed.version
void brokenVersion
void fixedVersion
EOF
"$tsc_bin" --strict --noEmit --ignoreConfig "$probe"
printf '%s\n' '--- relevant declarations and uses ---'
rg -n -C 4 \
'RequestedVersion|requestedVersion|type SlimPackument|interface SlimPackument|type SlimPackumentVersion|interface SlimPackumentVersion' \
app/composables/useRepositoryUrl.ts \
shared/types \
test/nuxt/composables/use-repository-url.spec.tsRepository: npmx-dev/npmx.dev
Length of output: 4554
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- RequestedVersion references ---'
rg -n -C 3 '\bRequestedVersion\b' app test shared
tsc_bin="${TSC_BIN:-tsc}"
command -v "$tsc_bin" >/dev/null
probe="$(mktemp --suffix=.ts)"
trap 'rm -f "$probe"' EXIT
cat >"$probe" <<'EOF'
type Version = { version: string; name: string; repository?: { url?: string } } | null
type Broken = Omit<Version, 'repository'> & { repository?: string }
type Fixed = Omit<Exclude<Version, null>, 'repository'> & { repository?: string }
const emptyBroken: Broken = {}
// `@ts-expect-error`: the broken type must not expose version.
const brokenVersion: string = ({} as Broken).version
// `@ts-expect-error`: the fixed type requires the version fields.
const emptyFixed: Fixed = {}
const fixed: Fixed = { version: '1.0.0', name: 'pkg' }
const fixedVersion: string = fixed.version
void emptyBroken
void brokenVersion
void emptyFixed
void fixedVersion
EOF
"$tsc_bin" --strict --noEmit --ignoreConfig "$probe"Repository: npmx-dev/npmx.dev
Length of output: 2112
Exclude null before applying Omit.
SlimPackument['requestedVersion'] is nullable. The current type removes its version fields and accepts {}. Use Omit<Exclude<SlimPackument['requestedVersion'], null>, 'repository'>.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/composables/useRepositoryUrl.ts` around lines 4 - 6, Update the
RequestedVersion type alias to exclude null from
SlimPackument['requestedVersion'] before applying Omit, using the existing
repository override and null union so the resulting type does not permit an
empty object from omitting fields on null.
There was a problem hiding this comment.
I'm already doing that
There was a problem hiding this comment.
@WilcoSp, the outer | null keeps the nullable result, but it does not exclude null before Omit.
The required change is:
export type RequestedVersion =
| (Omit<Exclude<SlimPackument['requestedVersion'], null>, 'repository'> & {
repository?: string | Repository
})
| nullThis preserves the requested-version fields and still permits null.
You are interacting with an AI system.
2 participants
🔗 Linked issue
I haven't found any yet
🧭 Context
I had noticed with date-fns that since version 4.3.0 that the links to github & changelog wasn't available
I've now at
useRepositoryUrl,detectChangelog, package.takumi.vue & package analysis changed parsing the repo url to allowpkg.repositoryto be either a string or an object.idk if this happens with more packages but at least with date-fns I did notice it.
📚 Description
What happened is that since date-fns 4.3.0 that the repository field of the meta data isn't being normalized anymore and is now the same as in package.json, (more info)
For
detectChangelog& package analysis I've changeparseRepoUrltoparseRepositoryInfoand given thepkg.repositorystring/objectFor
useRepositoryUrlI've added a check whether repo is a string and if it is then I normalized it and return the result, I had tests at first for this but removed them because the types changes needed would've made a lot for something that is a fallbackfor package.takumi.vue I've also add a type check for whether repository is a string or not
Also I do question whether we should do something with the
issueslink at the package page.what might have caused this
I think that this is caused due to the fact that the npm cli normalizes the repository field in package.json, but with pnpm v11 they've made their own publish command which might not normalize the repository
previews
date-fns 4.4.0
date-fns 4.2.1: