fix #231 filter no named imports

[edwinhuish]

Description 描述

related #231

解决当有非命名引用时，ts 转为 cjs 无法摇树，导致引入了 typescript 文件而出错的bug

Summary by CodeRabbit

• Bug Fixes

  • Improved page metadata processing by ignoring side-effect-only imports, preventing unintended side effects and producing more consistent results.

• Refactor

  • Streamlined import collection logic for better maintainability while preserving existing public behavior and interfaces.

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

📥 Commits

Reviewing files that changed from the base of the PR and between e802a2b and c619c3b.

📒 Files selected for processing (1)

• packages/core/src/page.ts (2 hunks)

 ___________________________________________________
< Stealth mode activated. Bugs won't see me coming. >
 ---------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

Tip

CodeRabbit can approve the review once all CodeRabbit's comments are resolved.

Enable the reviews.request_changes_workflow setting in your project's settings in CodeRabbit to automatically approve the review once all CodeRabbit's comments are resolved.

Walkthrough

Filters out side‑effect‑only imports from macro execution in tryPageMetaFromMacro by updating findImports to collect only ImportDeclaration nodes with specifiers. Public APIs remain unchanged; implementation shifts to an explicit loop for gathering imports.

Changes

Cohort / File(s)	Summary
Core: macro import handling packages/core/src/page.ts	Updated tryPageMetaFromMacro to exclude side‑effect imports from the macro import set. Rewrote findImports to use an explicit loop and return only ImportDeclarations with specifiers. No public API or type signature changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant P as Page Loader
  participant PM as tryPageMetaFromMacro
  participant FI as findImports
  participant EX as Macro Executor

  P->>PM: parse page source
  PM->>FI: collect imports (AST)
  FI-->>PM: ImportDeclarations with specifiers only
  PM->>EX: execute macro with { imports, code, filename }
  EX-->>PM: page meta (or error)
  PM-->>P: resolved page meta

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• chore: more configuration of typescript transpile module #228 — Also adjusts macro execution to pass an explicit imports array into execScript, affecting the same import collection and consumption path.

Suggested labels

size/XS

Poem

I sift through imports, hop by hop,
Side‑effects left behind—plop plop!
Only specs in my basket now,
Clean trails marked with a tidy bow.
Thump-thump, I code and cheer—
Meta found, the path is clear! 🐇✨

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

Pre-merge checks

❌ 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%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "fix #231 filter no named imports" directly references the linked issue and concisely describes the primary change—filtering non-named/side-effect imports—so it accurately reflects the raw_summary and PR objectives; it is therefore related and informative for reviewers, though it has minor grammar and capitalization issues.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

packages/core/src/page.ts (1)

170-177: Use Babel type guard for clarity and safer narrowing.

Minor readability/typing improvement; behavior unchanged.

-  for (const stmt of stmts) {
-    if (stmt.type !== 'ImportDeclaration') {
-      continue
-    }
-    imports.push(stmt)
-  }
+  for (const stmt of stmts) {
+    if (!t.isImportDeclaration(stmt))
+      continue
+    imports.push(stmt)
+  }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e43a37 and e802a2b.

📒 Files selected for processing (1)

• packages/core/src/page.ts (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

packages/core/src/page.ts (1)

packages/core/src/utils.ts (1)

• babelGenerate (144-144)

🔇 Additional comments (1)

packages/core/src/page.ts (1)

106-106: Restrict imports to runnable named value specifiers (exclude default/namespace/type-only)

Current filter only removes side‑effect imports and therefore still includes default/namespace imports. To ensure execScript receives only runnable value imports, change the filter to keep import declarations that are not type-only and that contain at least one ImportSpecifier (non-type); strip type-only specifiers from mixed imports.

Apply:

-    const imports = findImports(ast.body).filter(imp => !!imp.specifiers.length).map(imp => babelGenerate(imp).code)
+    const imports = findImports(ast.body)
+      // keep only value-bearing named imports; drop default/namespace-only and TS type-only
+      .filter(imp =>
+        imp.importKind !== 'type' &&
+        imp.specifiers.some(s => t.isImportSpecifier(s) && s.importKind !== 'type'),
+      )
+      .map((imp) => {
+        // strip type-only specifiers from mixed imports to ensure runnable JS
+        const valueSpecs = imp.specifiers.filter(s => !(t.isImportSpecifier(s) && s.importKind === 'type'))
+        if (valueSpecs.length !== imp.specifiers.length) {
+          const clone = t.cloneNode(imp) as t.ImportDeclaration
+          clone.specifiers = valueSpecs
+          return babelGenerate(clone).code
+        }
+        return babelGenerate(imp).code
+      })

Confirm intended behavior:

• Only exclude side‑effect imports (current code).
• Exclude default/namespace‑only imports as well (use diff above).

Add regression tests for:

• import 'x'
• import foo from 'x'
• import * as ns from 'x'
• import { a, type T } from 'x'
• import type { T } from 'x'

[file: packages/core/src/page.ts — around line 106]