Skip to content

fix: normalize component file imports - #418

Merged
DylanPiercey merged 1 commit into
mainfrom
normalize-component-file-import
Oct 23, 2025
Merged

fix: normalize component file imports#418
DylanPiercey merged 1 commit into
mainfrom
normalize-component-file-import

Conversation

@DylanPiercey

Copy link
Copy Markdown
Contributor

No description provided.

@changeset-bot

changeset-bot Bot commented Oct 23, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: d23dd16

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@marko/language-server Patch
@marko/language-tools Patch
@marko/type-check Patch
marko-vscode Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Oct 23, 2025

Copy link
Copy Markdown

Walkthrough

This pull request updates how component file imports are normalized across multiple Marko packages. The primary change modifies component type generation in the language-tools and language-server packages. Instead of importing and re-exporting component types as simple type aliases, the code now generates interface declarations that extend Marko._.ResolveComponent<typeof import("./component")>. This change is reflected in multiple test fixture snapshots for various component scenarios including class-based APIs and POJO components, as well as the code generation logic in packages/language-tools/src/extractors/script/index.ts. New test fixtures for POJO component auto-import scenarios are also added.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
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.
Description Check ❓ Inconclusive No pull request description was provided by the author. The description is neither related nor unrelated to the changeset; it is simply absent. Given that the evaluation criteria require either a related description, a completely unrelated description, or a vague/generic description to make a definitive determination, and an empty description does not clearly fit into any of these categories, there is insufficient information to conclusively determine whether this check passes or fails. Consider adding a pull request description that explains the rationale and impact of normalizing component file imports. This would help reviewers and future maintainers understand the purpose of the change and whether it affects component behavior, type safety, or developer experience.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title "normalize component file imports" directly relates to the core change in this pull request. The raw summary shows that the primary modification is in packages/language-tools/src/extractors/script/index.ts, where the code generation changes from importing and re-exporting a type alias (import type Component from "<path>"; export { type Component }) to emitting a public interface that properly resolves the component type (export interface Component extends ResolveComponent<typeof import("<path>")> {}). This normalization of component file imports is reflected throughout the test fixtures and snapshots. The title is clear, specific, and accurately captures the main intent of the changeset without vague language or misleading information.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch normalize-component-file-import

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ba56b21 and d23dd16.

📒 Files selected for processing (13)
  • .changeset/yellow-taxes-battle.md (1 hunks)
  • packages/language-server/src/__tests__/fixtures/script/class-api-with-component-auto-import/__snapshots__/class-api-with-component-auto-import.expected/index.md (1 hunks)
  • packages/language-server/src/__tests__/fixtures/script/class-api-with-component-auto-import/__snapshots__/class-api-with-component-auto-import.expected/index.ts (1 hunks)
  • packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/__snapshots__/class-api-with-pojo-component-auto-import.expected/index.html (1 hunks)
  • packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/__snapshots__/class-api-with-pojo-component-auto-import.expected/index.md (1 hunks)
  • packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/__snapshots__/class-api-with-pojo-component-auto-import.expected/index.ts (1 hunks)
  • packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/component.ts (1 hunks)
  • packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/index.marko (1 hunks)
  • packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/fancy-button/index.d.ts (1 hunks)
  • packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/fancy-button/index.ts (1 hunks)
  • packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/regular-button/index.d.ts (1 hunks)
  • packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/regular-button/index.ts (1 hunks)
  • packages/language-tools/src/extractors/script/index.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
packages/language-server/src/__tests__/fixtures/script/class-api-with-component-auto-import/__snapshots__/class-api-with-component-auto-import.expected/index.ts (2)
packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/__snapshots__/class-api-with-pojo-component-auto-import.expected/index.ts (1)
  • Component (2-3)
packages/language-tools/marko.internal.d.ts (1)
  • ResolveComponent (464-473)
packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/fancy-button/index.ts (5)
packages/language-server/src/__tests__/fixtures/script/class-api-with-component-auto-import/__snapshots__/class-api-with-component-auto-import.expected/index.ts (1)
  • Component (2-3)
packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/__snapshots__/class-api-with-pojo-component-auto-import.expected/index.ts (1)
  • Component (2-3)
packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/fancy-button/index.d.ts (1)
  • Component (2-3)
packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/regular-button/index.ts (1)
  • Component (2-3)
packages/language-tools/marko.internal.d.ts (1)
  • ResolveComponent (464-473)
packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/regular-button/index.d.ts (6)
packages/language-server/src/__tests__/fixtures/script/class-api-with-component-auto-import/__snapshots__/class-api-with-component-auto-import.expected/index.ts (1)
  • Component (2-3)
packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/__snapshots__/class-api-with-pojo-component-auto-import.expected/index.ts (1)
  • Component (2-3)
packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/fancy-button/index.d.ts (1)
  • Component (2-3)
packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/fancy-button/index.ts (1)
  • Component (2-3)
packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/regular-button/index.ts (1)
  • Component (2-3)
packages/language-tools/marko.internal.d.ts (1)
  • ResolveComponent (464-473)
packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/fancy-button/index.d.ts (4)
packages/language-server/src/__tests__/fixtures/script/class-api-with-component-auto-import/__snapshots__/class-api-with-component-auto-import.expected/index.ts (1)
  • Component (2-3)
packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/fancy-button/index.ts (1)
  • Component (2-3)
packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/regular-button/index.d.ts (1)
  • Component (2-3)
packages/language-tools/marko.internal.d.ts (1)
  • ResolveComponent (464-473)
packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/regular-button/index.ts (4)
packages/language-server/src/__tests__/fixtures/script/class-api-with-component-auto-import/__snapshots__/class-api-with-component-auto-import.expected/index.ts (1)
  • Component (2-3)
packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/fancy-button/index.ts (1)
  • Component (2-3)
packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/regular-button/index.d.ts (1)
  • Component (2-3)
packages/language-tools/marko.internal.d.ts (1)
  • ResolveComponent (464-473)
packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/__snapshots__/class-api-with-pojo-component-auto-import.expected/index.ts (1)
packages/language-tools/marko.internal.d.ts (5)
  • ResolveComponent (464-473)
  • Template (49-54)
  • Directives (4-4)
  • Relate (475-475)
  • ReturnWithScope (73-76)
🪛 LanguageTool
packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/__snapshots__/class-api-with-pojo-component-auto-import.expected/index.md

[grammar] ~2-~2: Il y a peut-être une erreur ici
Context: ## Hovers ### Ln 3, Col 15 marko 1 | <div onClick("handleClickMissing")/> 2 | <div onClick("strange-handler")/> > 3 | $ console.log(component); | ^ const component: Component 4 | // ^? ## Diagnostics ### Ln 1, Col 15 ```marko > ...

(QB_NEW_FR)


[grammar] ~12-~12: Il y a peut-être une erreur ici
Context: ... ^? ## Diagnostics ### Ln 1, Col 15marko > 1 | <div onClick("handleClickMissing")/> | ^^^^^^^^^^^^^^^^^^ Property 'handleClickMissing' does not exist on type 'Component'. 2 | <div onClick("strange-handler")/> 3 | $ console.log(component); 4 | // ^? ```

(QB_NEW_FR)

🔇 Additional comments (13)
.changeset/yellow-taxes-battle.md (1)

1-8: LGTM!

The changeset documentation clearly describes the change and appropriately marks all affected packages with patch-level version bumps.

packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/index.marko (1)

1-4: LGTM!

This test fixture appropriately tests both error cases (missing handler) and valid cases (existing handler), along with component type inference.

packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/__snapshots__/class-api-with-pojo-component-auto-import.expected/index.html (1)

1-1: LGTM!

The HTML snapshot correctly represents the expected output for the test fixture.

packages/language-server/src/__tests__/fixtures/script/class-api-with-component-auto-import/__snapshots__/class-api-with-component-auto-import.expected/index.md (1)

1-19: LGTM!

The updated diagnostic message now correctly references the Component type instead of default, which aligns with the interface-based approach introduced in this PR.

packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/component.ts (1)

1-4: LGTM!

This POJO component fixture appropriately tests both standard method names and quoted property names for method handlers.

packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/__snapshots__/class-api-with-pojo-component-auto-import.expected/index.md (1)

1-20: LGTM!

The snapshot correctly documents the expected hovers and diagnostics for the POJO component auto-import test case, showing proper type inference and error messages.

packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/regular-button/index.d.ts (1)

2-3: LGTM! Core pattern change looks solid.

The shift from a type alias to an interface extending Marko._.ResolveComponent provides more robust type resolution by properly handling default exports, instance types, and component types. This pattern is consistently applied across all affected snapshots.

packages/language-server/src/__tests__/fixtures/script/class-api-with-component-auto-import/__snapshots__/class-api-with-component-auto-import.expected/index.ts (1)

2-3: LGTM!

Consistent application of the interface pattern for Component type resolution. The change maintains backward compatibility while providing more robust type handling.

packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/fancy-button/index.ts (1)

2-3: LGTM! Component interface normalized.

The change from a type alias to an interface extending ResolveComponent normalizes component type handling across the codebase. This approach provides consistent type resolution while allowing for future extensibility.

packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/fancy-button/index.d.ts (1)

2-3: LGTM! Consistent type declaration.

The declaration file correctly mirrors the interface pattern used in the implementation files, ensuring type consistency across the package.

packages/language-server/src/__tests__/fixtures/script/split-component-d-ts/__snapshots__/split-component-d-ts.expected/components/regular-button/index.ts (1)

2-3: LGTM! Handles browser-specific component correctly.

The interface pattern correctly resolves the browser-specific component file, demonstrating the flexibility of the normalization approach across different component file variants.

packages/language-tools/src/extractors/script/index.ts (1)

273-275: LGTM! Core normalization logic implemented correctly.

This generates the normalized component interface pattern for external component files. The implementation correctly:

  • Uses ResolveComponent utility for type resolution
  • Strips file extensions for clean imports
  • Generates relative import paths
  • Creates an empty interface that extends the resolved type
packages/language-server/src/__tests__/fixtures/script/class-api-with-pojo-component-auto-import/__snapshots__/class-api-with-pojo-component-auto-import.expected/index.ts (1)

2-3: LGTM! POJO component integration verified.

The normalized component interface pattern works correctly for POJO components with auto-import, demonstrating the approach's versatility across different component types.


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.

@DylanPiercey
DylanPiercey merged commit 67ae86c into main Oct 23, 2025
4 checks passed
@DylanPiercey
DylanPiercey deleted the normalize-component-file-import branch October 23, 2025 23:38
@github-actions github-actions Bot mentioned this pull request Oct 23, 2025
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.

1 participant