Skip to content

fix: stop leaking the internal "~api" export into .d.marko output - #584

Merged
DylanPiercey merged 1 commit into
mainfrom
dpiercey-ws-marko-mtc-issue-pl7ym4
Aug 4, 2026
Merged

fix: stop leaking the internal "~api" export into .d.marko output#584
DylanPiercey merged 1 commit into
mainfrom
dpiercey-ws-marko-mtc-issue-pl7ym4

Conversation

@DylanPiercey

@DylanPiercey DylanPiercey commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

The standalone "~api" export added to the extracted script in e986bcb was being copied into generated .d.marko files, which came out as static declare const __marko_internal_api = "class"; plus the export itself. The .d.marko printer now skips it and the const backing it — matching the generated export's local name and rejecting re-exports, so a source export { something as "~api" } is left alone. The api is re-derived whenever the .d.marko is extracted, so nothing is lost.

Also fixes isVariableStatementForName comparing escapedText, which TypeScript prefixes with an extra underscore for __-prefixed identifiers.

Adds the coverage that was missing: @marko/type-check now snapshots every emitted .d.marko for a fixture project and type checks the emitted files as Marko sources, so leaked internals fail the build.

@changeset-bot

changeset-bot Bot commented Aug 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 21e4375

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

This PR includes changesets to release 5 packages
Name Type
@marko/language-server Patch
@marko/language-tools Patch
@marko/ts-plugin 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 Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The change centralizes generated runtime API identifiers and filters the internal API variable and "~api" export from generated .d.marko declarations. It adds a patch changeset for this behavior. The type-check package gains Mocha commands, configuration, declaration-emission tests, Marko fixtures, and a declaration-only TypeScript configuration.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check ✅ Passed The title clearly and concisely describes the main fix to prevent the internal "~api" export from leaking into .d.marko output.
Description check ✅ Passed The description directly explains the output fix, identifier matching correction, and added test coverage.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dpiercey-ws-marko-mtc-issue-pl7ym4

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@agent-feedback/dx.md`:
- Around line 4-15: Record the actionable declaration-emit test-coverage finding
from agent-feedback/dx.md in agent-feedback/README.md. Preserve the requested
scope and details, including the mtc fixture/snapshot validation, optional
re-check of emitted files, and verification of whether the recursive test
command covers packages/type-check.

In `@packages/language-tools/src/processors/marko.ts`:
- Around line 175-176: Update the filters around isInternalApiVar and
isExportInternalApi to match only generated internal API declarations: require
the exported property name to equal INTERNAL_API_VAR and ensure moduleSpecifier
is absent, excluding source exports and re-exports such as userApi. Apply
equally specific checks to isInternalApiVar so user-authored declarations are
preserved.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 247ad7ea-eb69-401b-b63c-b422de40faed

📥 Commits

Reviewing files that changed from the base of the PR and between 0c2ea1d and ed25f44.

📒 Files selected for processing (4)
  • .changeset/dts-api-export.md
  • agent-feedback/dx.md
  • packages/language-tools/src/extractors/script/index.ts
  • packages/language-tools/src/processors/marko.ts

Comment thread agent-feedback/dx.md Outdated
Comment thread packages/language-tools/src/processors/marko.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/type-check/package.json`:
- Around line 30-31: Update the package.json test and test:update scripts to run
the repository’s dependency-aware build command before invoking Mocha, ensuring
`@marko/language-tools` is built before type-check. Preserve the existing test
glob and update flag after the build step.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 92986839-9e21-4f41-9076-cf261d8e6c7f

📥 Commits

Reviewing files that changed from the base of the PR and between ed25f44 and ed11dab.

⛔ Files ignored due to path filters (3)
  • packages/type-check/src/__tests__/fixtures/emit/__snapshots__/emits-declaration-files.expected/counter.d.marko is excluded by !**/__snapshots__/** and included by **
  • packages/type-check/src/__tests__/fixtures/emit/__snapshots__/emits-declaration-files.expected/greet.d.marko is excluded by !**/__snapshots__/** and included by **
  • packages/type-check/src/__tests__/fixtures/emit/__snapshots__/emits-declaration-files.expected/static.d.marko is excluded by !**/__snapshots__/** and included by **
📒 Files selected for processing (7)
  • packages/type-check/.mocharc.json
  • packages/type-check/package.json
  • packages/type-check/src/__tests__/emit.test.ts
  • packages/type-check/src/__tests__/fixtures/emit/src/counter.marko
  • packages/type-check/src/__tests__/fixtures/emit/src/greet.marko
  • packages/type-check/src/__tests__/fixtures/emit/src/static.marko
  • packages/type-check/src/__tests__/fixtures/emit/tsconfig.json

Comment thread packages/type-check/package.json
@DylanPiercey
DylanPiercey force-pushed the dpiercey-ws-marko-mtc-issue-pl7ym4 branch from f043eee to 8499935 Compare August 4, 2026 20:54
The standalone `"~api"` export added to the extracted script was being copied
into generated `.d.marko` files as `static declare const __marko_internal_api`.
The `.d.marko` printer now skips it and the `const` backing it, matching on the
generated export's local name and rejecting re-exports so a source
`export { something as "~api" }` is left alone. The api is re-derived whenever
the `.d.marko` is itself extracted, so nothing is lost.

Also fixes `isVariableStatementForName` comparing `escapedText`, which
TypeScript prefixes with an extra underscore for `__`-prefixed identifiers.

Adds the missing coverage: `@marko/type-check` now snapshots every emitted
`.d.marko` for a fixture project and type checks the emitted files as Marko
sources, so leaked internals fail the build.
@DylanPiercey
DylanPiercey force-pushed the dpiercey-ws-marko-mtc-issue-pl7ym4 branch from 8499935 to 21e4375 Compare August 4, 2026 21:01
@DylanPiercey
DylanPiercey merged commit 4e2cedc into main Aug 4, 2026
5 checks passed
@DylanPiercey
DylanPiercey deleted the dpiercey-ws-marko-mtc-issue-pl7ym4 branch August 4, 2026 21:04
@github-actions github-actions Bot mentioned this pull request Aug 4, 2026
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