Visitar URL original
fix: only strip known module extensions by danielroe · Pull Request #577 · unjs/unimport · GitHub
Skip to content

fix: only strip known module extensions - #577

Open
danielroe wants to merge 1 commit into
mainfrom
fix/strip-file-extension
Open

fix: only strip known module extensions#577
danielroe wants to merge 1 commit into
mainfrom
fix/strip-file-extension

Conversation

@danielroe

@danielroe danielroe commented Oct 12, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

when registering an extensionless import with a dotted suffix (example from my work on nuxt-modules/turnstile#713):

addServerImports({
  name: 'verifyTurnstileToken',
  from: '/abs/src/runtime/server/utils/verify.portable',
})

the generated re-export points at a different (real) file:

export { verifyTurnstileToken } from '../../../src/runtime/server/utils/verify';

this is because stripFileExtension and toExports treat any dotted segment as an extension.

this PR narrows the regex to the module extensions scanDirExports looks up, plus .vue. .d.ts paths still become .d, and other suffixes like .json or .svelte are now kept

Summary by CodeRabbit

  • Bug Fixes
    • File paths with dotted names are now preserved unless they end in a supported JavaScript, TypeScript, or Vue extension. Export and import paths handle these names more accurately.
  • Tests
    • Added coverage for dotted path segments and extension handling in export and type declaration paths.

@danielroe
danielroe requested a review from antfu October 12, 2026 13:17
@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2ec2d8a0-498d-4ff8-bbf8-350b79b2af10
📥 Commits

Reviewing files that changed from the base of the PR and between bbd0776 and 3599c12.

📒 Files selected for processing (3)
  • src/utils.ts
  • test/to-export.test.ts
  • test/to-imports.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Walkthrough

The extension pattern now matches supported JavaScript, TypeScript, and Vue suffixes, including c and m variants. Regression tests cover dotted path segments in generated exports, type re-exports, and declaration-file imports.

Changes

Extension matching

Layer / File(s) Summary
Extension matching and path regressions
src/utils.ts, test/to-export.test.ts, test/to-imports.test.ts		 The extension pattern now recognizes supported JavaScript, TypeScript, and Vue suffixes instead of any alphabetic suffix. Tests cover preservation of dotted non-extension segments and removal of recognized suffixes in export and import paths.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk | ⚪ Minimal · up to 3599c

Merge Risk: ⚪ Minimal · up to 3599c

The extension change appears ready to merge after normal checks. Supported module suffixes remain recognized, while unrelated dotted suffixes are preserved.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 3599c

The change preserves dotted module names instead of redirecting them to different files. It does not introduce a new input source, module-selection privilege, or security boundary.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is downstream module references generated from caller- or scanner-provided paths. The comparison does not introduce tenant, credential, network, or deployment authority changes; behavior in external consuming applications was not inspected.

Trust Boundaries and Controls

  • inferred — Controlling Import.from already permits caller-selected module identifiers. Extension stripping is normalization, not an authorization allowlist. Preserving unsupported dotted suffixes therefore does not demonstrate a new module-selection privilege or bypass of an existing security control.
Pre-merge checks | Passed 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: limiting extension stripping to known module extensions.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
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.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/utils.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

test/to-export.test.ts

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).

test/to-imports.test.ts

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).

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.

@codecov

codecov Bot commented Oct 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.49%. Comparing base (bbd0776) to head (3599c12).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #577   +/-   ##
=======================================
  Coverage   94.49%   94.49%           
=======================================
  Files          20       20           
  Lines        1180     1180           
  Branches      397      397           
=======================================
  Hits         1115     1115           
  Misses         59       59           
  Partials        6        6

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@antfu antfu Awaiting requested review from antfu

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@danielroe