fix: get url from pkg.repository when string - #3173
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 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
🚥 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.
another solution maybe could be to patch the @npm/types to add the string for repository
There was a problem hiding this comment.
I've now changed RequestedVersion to have | {repository: string} which will add string as an option, this is I think the closest to what the type should be without doing a patch to @npm/types
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
… what was expected
gameroman
added
the
needs review
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 also added the possibility of string repository atRequestedVersiontypefor 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: