Skip to content

fix(types): make StaticImport/TypeImport imports optional - #362

Open
MFA-G wants to merge 1 commit into
unjs:mainfrom
MFA-G:fix/static-import-imports-optional
Open

fix(types): make StaticImport/TypeImport imports optional#362
MFA-G wants to merge 1 commit into
unjs:mainfrom
MFA-G:fix/static-import-imports-optional

Conversation

@MFA-G

@MFA-G MFA-G commented Aug 10, 2026

Copy link
Copy Markdown

Fixes #349

Problem

ESM_STATIC_IMPORT_RE has an optional imports capture group, because a side effect import has no import clause:

import\s*(?:[\s"']*(?<imports>...)from\s*)?["']...
                                          ^ optional

So for a side effect import the property is simply absent at runtime:

findStaticImports('import "styles.css";')[0].imports // => undefined

But StaticImport['imports'] (and TypeImport['imports']) declared it as a required string. TypeScript therefore told consumers the value was always present, and code like match.imports.includes("type") type-checked cleanly while throwing at runtime.

The existing suite has a side effect import case, but it only asserts the specifier, so it never caught the mismatch.

Fix

  • imports is now imports?: string on StaticImport and TypeImport, with a doc comment explaining when it is undefined.
  • clearImports accepts string | undefined. Its body already did (imports || ""), so this only makes the signature match the behaviour it already had — parseStaticImport / parseTypeImport keep working unchanged on side effect imports.
  • findTypeImports guards the filter with match.imports ?? "", which was the one place that fed the possibly-undefined value straight into RegExp.test (it previously relied on test stringifying undefined to "undefined").
  • Added a regression test asserting imports is undefined and that parseStaticImport still yields empty namedImports.

Compatibility

This is a type-only change for a value that was already undefined at runtime. It is strictly more accurate, though downstream code that assumed a non-optional string under strict will now surface the guard it was always missing — which is the point of the issue.

Validation

  • vitest run — 199/199 pass (6 files)
  • tsc --noEmit — clean
  • eslint src test — clean
  • prettier --check src test — clean
  • unbuild — builds successfully

Summary by CodeRabbit

  • New Features

    • Added support for side-effect imports without named import clauses.
    • Import analysis now safely handles missing import details.
  • Bug Fixes

    • Improved handling of undefined import values while preserving existing cleanup behavior.
  • Tests

    • Added coverage confirming side-effect imports are represented correctly and parsed with no named imports.

Side effect imports such as `import "styles.css"` have no import clause,
so the `imports` named capture group of ESM_STATIC_IMPORT_RE is undefined.
The type declared it as a required `string`, so consumers had no indication
they needed a guard and were free to call string methods on undefined.

Mark `imports` optional on both interfaces, widen `clearImports` to accept
`undefined` (its body already coerced falsy input to ""), guard the
`findTypeImports` filter, and add a regression test.

Fixes unjs#349
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: aed10620-ccad-463c-a37d-fea41fefb99c

📥 Commits

Reviewing files that changed from the base of the PR and between beece9b and ef8eb3a.

📒 Files selected for processing (3)
  • src/_utils.ts
  • src/analyze.ts
  • test/imports.test.ts

📝 Walkthrough

Walkthrough

The import analysis types now allow missing import clauses for side-effect imports. Utility handling accepts undefined values, type-import filtering uses a safe fallback, and tests verify the parsed side-effect import shape.

Changes

Side-effect import support

Layer / File(s) Summary
Optional import clause contracts
src/analyze.ts, src/_utils.ts
StaticImport.imports and TypeImport.imports are optional. clearImports accepts undefined values. Type-import filtering handles missing clauses with an empty fallback.
Side-effect import validation
test/imports.test.ts
Tests verify that import "styles.css"; has an undefined raw import clause and an empty parsed named-import map.

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

🚥 Pre-merge checks | ✅ 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 identifies the primary change: making StaticImport and TypeImport imports optional.
Linked Issues check ✅ Passed The changes address issue #349 by making imports optional, handling undefined values, and adding regression coverage for side-effect imports.
Out of Scope Changes check ✅ Passed All code and test changes support the linked issue and stated objective; no unrelated changes are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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.

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.

The StaticImport['imports'] type is incorrect

1 participant