fix: no types declaration in package.json#1674
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
package.jsonpackage.json
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughAdds file-tree-aware package analysis: the API now optionally fetches and flattens a package file tree and concurrently fetches 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.
🧹 Nitpick comments (4)
test/unit/shared/utils/package-analysis.spec.ts (1)
181-211: Rename the “path derivation” cases to match what is actually asserted.These tests currently validate declaration-file presence in
files, not explicit derivation from runtime entrypoints. Renaming will avoid misleading future readers.Suggested naming-only diff
-describe('detectTypesStatus implicit types (path derivation)', () => { - it('derives .d.mts from .mjs in exports', () => { +describe('detectTypesStatus implicit declaration files', () => { + it('detects .d.mts in files', () => { @@ - it('derives .d.cts from .cjs in exports', () => { + it('detects .d.cts in files', () => { @@ - it('derives .d.mts from main when type is module', () => { + it('detects .d.mts in files when package is module', () => {server/api/registry/analysis/[...pkg].get.ts (2)
60-67: Run@typeslookup and file-tree fetch concurrently to reduce endpoint latency.Both calls are independent in this branch, so serial awaits add avoidable response time.
Suggested refactor (keeps file-tree failure tolerance)
if (!hasBuiltInTypes(pkg)) { const typesPkgName = getTypesPackageName(packageName) - typesPackage = await fetchTypesPackageInfo(typesPkgName) - const resolvedVersion = pkg.version ?? version ?? 'latest' - try { - const fileTreeResponse = await getPackageFileTree(packageName, resolvedVersion) - files = flattenFileTree(fileTreeResponse.tree) - } catch { - // File tree fetch failed - skip implicit types check - } + const typesPackagePromise = fetchTypesPackageInfo(typesPkgName) + const filesPromise = getPackageFileTree(packageName, resolvedVersion) + .then(fileTreeResponse => flattenFileTree(fileTreeResponse.tree)) + .catch(() => undefined) + + typesPackage = await typesPackagePromise + files = await filesPromise }
67-69: Consider lightweight telemetry in the file-tree failure path.The empty catch keeps behaviour resilient, but it also hides recurring upstream issues. A debug log/metric here would help monitor implicit-types coverage.
shared/utils/package-analysis.ts (1)
249-253: Align the inline comment with the implemented behaviour.The current code checks for any declaration file in
files; it does not collect entrypoint-derived paths fromexports/main/module. Please update the comment to prevent misinterpretation.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
server/api/registry/analysis/[...pkg].get.tsshared/utils/package-analysis.tstest/unit/shared/utils/package-analysis.spec.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shared/utils/package-analysis.ts (1)
267-280: Consider handling extensionless or non-JS entry paths.The function returns an empty array when the entry path doesn't end with a known JS extension (
.mjs,.cjs,.js). This is a safe fallback, but packages may occasionally specify entry points with unusual patterns (e.g., extensionless paths or.tsentries in rare cases). The current behaviour is acceptable for the PR's objective, but it's worth noting this limitation.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
server/api/registry/analysis/[...pkg].get.tsshared/utils/package-analysis.tstest/unit/shared/utils/package-analysis.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/unit/shared/utils/package-analysis.spec.ts
🔗 Linked issue
Fixed #1669
🧭 Context
npmx currently shows “no types” for packages that ship declaration files via implicit lookup (without types/typings in package.json), even when those files exist in the package.
📚 Description
Check the files that it include some file like
*.d.ts,*.d.ctsor*.d.mts, then we think the package has types.Result