Skip to content

fix(types): emit auto-import paths as files, not directories#4333

Merged
pi0 merged 1 commit into
nitrojs:mainfrom
danielroe:fix/deno-types
Jun 10, 2026
Merged

fix(types): emit auto-import paths as files, not directories#4333
pi0 merged 1 commit into
nitrojs:mainfrom
danielroe:fix/deno-types

Conversation

@danielroe

@danielroe danielroe commented Jun 10, 2026

Copy link
Copy Markdown
Member

🔗 Linked issue

originally reported against Nuxt: nuxt/nuxt#35248, but it reproduces in pure Nitro
also related: #1528, #4297

❓ Type of change

  • 📖 Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

writeTypes emits const foo: typeof import('<path>').foo per auto-imported binding

for some packages, the path ends up being the package directory rather than a file; this works fine for TypeScript with moduleResolution: bundler, but Deno's TS host doesn't apply Node directory resolution to relative specifiers and bails

this PR treats ./ the same as an empty subpath & falls back to resolvedPath, which ends up having its extension stripped

both typescript + deno should resolve this equally well.

note: packages with different exports.types vs exports.imports (eg #2384) won't work correctly, either before or after this fix (but the error slightly changes 😆)

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@danielroe danielroe requested a review from pi0 as a code owner June 10, 2026 13:24
@vercel

vercel Bot commented Jun 10, 2026

Copy link
Copy Markdown

@danielroe is attempting to deploy a commit to the Nitro Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR fixes node module path resolution in Nitro's writeTypes function. When generating type declarations, the import path logic now uses lookupNodeModuleSubpath to compute subpaths and conditionally joins directory components only when the subpath is meaningful (not "./"), otherwise falling back to the previously resolved path. A new test validates this behavior with packages defining exports only at ".".

Changes

Node module subpath resolution in type declarations

Layer / File(s) Summary
Path resolution fix and test validation
src/build/types.ts, test/unit/types-imports.test.ts
writeTypes now uses lookupNodeModuleSubpath to determine if a subpath should be joined to the directory path; a new test verifies that import specifiers reference file paths rather than package directories for exports-only packages.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(types): emit auto-import paths as files, not directories' follows conventional commits format with a clear 'fix' type prefix and descriptive scope.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description clearly relates to the changeset, explaining the bug fix for emitted auto-import paths and the exact changes made to resolve type path handling in both TypeScript and Deno.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@pkg-pr-new

pkg-pr-new Bot commented Jun 10, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/nitro@4333

commit: 1700580

@pi0 pi0 merged commit 2395894 into nitrojs:main Jun 10, 2026
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants