Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/semantic-tokens.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@marko/language-server": minor
"@marko/language-tools": patch
"marko-vscode": minor
---

Add semantic token support (`textDocument/semanticTokens` full and range). Custom component tags stop looking like native HTML — `<my-button>` and `<MyButton>` both render in the component/type color, matching how `.tsx` files color `<MyComponent/>`, while `<div>` keeps its tag color. Core tags the grammar doesn't recognize (`<attrs>`, `<effect>`, `<macro>`) pick up core-tag coloring, and identifiers in placeholders, attribute values, tag params/vars, and `static`/`server`/`class` blocks are colored from TypeScript's view of the code. Core-tag detection is now shared with tag-name completions, which as a side effect improves completion documentation for core tags resolved through nested `node_modules`.
60 changes: 60 additions & 0 deletions agent-feedback/bugs.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,68 @@ The runtime accepts a string OR number loop key — `assertValidLoopKey` throws

A custom tag that fails to resolve (a component referenced by kebab tag name with no `import`, or a typo'd tag name) is a hard build error but produces ZERO diagnostics under `mtc` — the type-check tool agents are told to run. Reproduced in a real @marko/run scaffold: Marko 6 auto-discovers only `tags/` dirs (`runtime-tags` sets `tagDiscoveryDirs = ["tags"]` at `packages/runtime-tags/src/translator/index.ts:40`), so `src/components/*.marko` is NOT auto-registered — `<user-card name="X" age="alsoWrong" bogus=true/>` (no import) and the typo `<char-cont max="wrong" nope=1/>` both pass `mtc` with exit 0, while `npm run compile -o html` / `marko-run build` throw `Unable to find entry point for custom tag <user-card>` (`resolveTagImport` at `tags.js:353`; `tagNotFoundError` at `packages/runtime-tags/src/translator/visitors/tag/custom-tag.ts:411`). The identical wrong attr on a RESOLVED tag correctly errors TS2322 (verified against `<char-count max="not-a-number">`), proving tag resolution — not the attribute — is the gate: `#writeTag` lowers an unresolvable tag to `renderDynamicTag(...)` whose input is `Record<string, unknown>`, so every attribute and callback param goes unchecked. This is the worst shape for an agent whose deterministic verify loop is `mtc`: it creates a component, references it by tag (natural for anyone used to auto-registering `components/` dirs), sees a clean type-check, and ships wrong props or a misspelled tag; the build then fails with an "entry point" error that looks unrelated to the type loop. Direction: have @marko/language-tools emit a distinct "cannot resolve tag `<x>`" diagnostic mirroring the compiler instead of degrading to an untyped dynamic tag. The compiler-side authority is marko-js/marko's `packages/compiler/src/babel-utils/tags.js` › `resolveTagImport` and `custom-tag.ts` › `tagNotFoundError`. Distinct from the run dx.md route-types entries (missing `Run` global / stale `routes.d.ts`), which concern generated route types, not custom-tag resolution.

## Guard `Extracted.sourceRangeAt` consumers against anchor-token expansion and cross-token spans

`packages/language-tools/src/util/extractor.ts` › `Extracted` | 2026-07-27 | impact:med | effort:med

Two hazards exist for a consumer that maps a generated range back to source and assumes the length is preserved. A zero-width `Mapping.anchor` token (`generatedLength === 0`, `sourceLength > 0`) matches a query at exactly its `generatedStart`, and `rangeAt`/`sourceRangesAt` then contribute the anchor's entire unrelated source range. Separately, `rangeAt` pairs the first overlapping start token with the last end token, spanning any unmapped glue between two different tokens, so the returned source range can be longer than and textually different from the queried generated span; `sourceLocationAtTextSpan` in `packages/language-server/src/service/script/index.ts` relies on `sourceLocationAt` and can therefore report oversized or misplaced ranges for TS results whose spans straddle mappings. Consumers that need length preservation should use `sourceRangesAt`, which returns every overlapping mapping, and validate per feature: semantic tokens require a single result of exact width with byte-identical text, while highlights legitimately accept each returned range. Alternatively `rangeAt` could learn to reject cross-token spans. Re-verify: query `sourceRangeAt` over a generated span that begins at an `anchor` offset (the anchors emitted in `extractors/script/index.ts` around tag names) and compare the returned width to the queried width.

## Add `macro` and `effect` to the TextMate grammar's core-tag keyword lists

`packages/vscode/syntaxes/marko.tmLanguage.json` › `#tag-name` | 2026-07-27 | impact:low | effort:low

The `#tag-name` rule hardcodes two alternations for Marko core tags (`for|if|while|else-if|else|try|await|return` scoped `keyword.control.flow.marko` and `const|context|debug|define|id|let|log|lifecycle` scoped `support.function.marko`), and `macro` and `effect` appear in neither, so both color as ordinary user-defined tags (`entity.name.tag.marko`). `grep -n "macro" packages/vscode/syntaxes/marko.tmLanguage.json` returns no hits today, and both are real core tags: `macro` is recognized in `packages/language-tools/src/extractors/script/util/get-runtime-api.ts`. The fix is adding the two names to the alternations. Re-verify: open a `.marko` file containing `<macro|...|/foo>` and check the scope with VS Code's "Developer: Inspect Editor Tokens and Scopes".

## Keep the `"use strict"` prologue when injecting the `import.meta.url` banner

`packages/ts-plugin/build.mts` › `build` | 2026-07-24 | impact:low | effort:low

The `banner.js` that defines `_importMetaUrl` is emitted above esbuild's own `"use strict"`, so that string is no longer a directive prologue and the bundle runs in sloppy mode: `head -2 packages/ts-plugin/dist/index.js` shows the `const` on line 1 and `"use strict";` on line 2. The same applies to all three `packages/vscode/dist/*.js` bundles, which share the pattern via `packages/vscode/build.mts`. Bundled TS rarely depends on strict semantics, so nothing is known to break today, but silently dropping strict mode is not what either build intends. Fix by prefixing the banner with `"use strict";\n` (as `packages/language-tools/build.mts` does); re-verify with `head -2` on each bundle.

## Make `clearMarkoCacheForFile` clear the same cache `getMarkoFile` populates for non-file documents

`packages/language-server/src/utils/file.ts` › `clearMarkoCacheForFile` | 2026-07-28 | impact:med | effort:low

`getMarkoFile` caches a non-`file:` (e.g. `untitled:`) document under `Project.getCache(process.cwd())`, but `clearMarkoCacheForFile` deletes from `Project.getCache(getFSDir(doc))`, which is `getCache(undefined)` for those documents. When the cwd resolves a local marko install those are different maps, the delete is a no-op, and edits to untitled documents keep serving the stale parse; reproduced live, where document symbols requested after an edit that inserted a new tag still returned only the pre-edit symbols. Fix by extracting one shared helper that computes the cache directory identically for both call sites. Re-verify: open an untitled marko doc, request symbols, apply an edit adding a tag, and request symbols again; the new tag is currently missing.

## Invalidate the compiler-diagnostics cache on watched-file changes, not just document versions

`packages/language-server/src/service/marko/validate.ts` › `getMarkoDiagnostics` | 2026-07-28 | impact:med | effort:low

The compiler diagnostics cache is keyed per `(doc, doc.version)`, but a watched-file event (e.g. creating the component a template references) triggers revalidation without changing the document's version, so stale cached diagnostics such as an unresolved-tag error the on-disk change just fixed are re-sent until the user edits the document itself. Key the cache additionally on a cache generation such as `documents.projectVersion`, or clear it from the watched-files path. Re-verify: open a doc referencing a missing component, create the component file on disk, fire the watched-files event, and re-validate; the error should clear without editing the doc.

## Recover from a whitespace-containing close tag instead of swallowing the rest of the file

`packages/language-tools/src/parser.ts` › `Builder` | 2026-07-28 | impact:med | effort:med

A close tag with trailing whitespace before `>` (e.g. `</if >`) leaves the close range `UNFINISHED` and the parser absorbs the entire remainder of the file into it, so every feature that walks the CST (symbols, links, semantic tokens) loses everything below that point; mid-keystroke this visibly strips highlighting for the rest of the file. The runtime compiler accepts the same input, so the gap is in parser recovery rather than in the language definition. Re-verify: parse `<if=1>a</if >\n<for|x| of=[1]>${x}</for>` and observe that the `<for>` tag is missing from `program.body` because it lives inside the `if` close range.

## Bound recursion (or go iterative) in the script extractor and remaining CST walkers

`packages/language-tools/src/extractors/script/index.ts` › `ScriptExtractor` | 2026-07-28 | impact:low | effort:med

Roughly 5,000 levels of nested unclosed tags overflow the call stack in the recursive script extractor, as well as in the remaining recursive walkers in `packages/language-server/src/service/marko/document-symbols.ts` and `document-links.ts`, and the facade's `Promise.allSettled` swallows the `RangeError`, so features return empty results for the document with no error surfaced. The semantic-token walkers were converted to explicit stacks during review; the extractor and the two remaining walkers still recurse. Re-verify: use `"<if=1>".repeat(5000)` as a document body and request document symbols, which currently returns empty with a swallowed RangeError.

## Wrap plugin calls in `createService` merge arms so synchronous throws are isolated

`packages/language-server/src/service/create-service.ts` › `createService` | 2026-07-28 | impact:low | effort:low

Every merge arm except `getSemanticTokens` calls `plugin.method?.(...)` directly inside the `plugins.map(...)` passed to `Promise.allSettled`, so a plugin method that throws synchronously (several script-plugin handlers are synchronous) throws out of the `.map` callback before `allSettled` can isolate it, rejecting the entire feature instead of that one plugin's contribution. `getSemanticTokens` wraps the call in an `async` closure to isolate such throws; the same one-word change applies to the other arms. Re-verify: a test plugin whose `findDocumentHighlights` throws synchronously currently rejects the whole facade call rather than being skipped.

## Classify mutation targets of tag variables in the extracted script

`packages/language-tools/marko.internal.d.ts` › `change` | 2026-07-28 | impact:low | effort:med

The assignment target of a mutated `<let>` variable is rewritten to a generated property access (`__marko_internal_change__x.x = ...`), and TypeScript emits no semantic classification for that property because the `Marko._.change(...)` type does not resolve it to a named property symbol, even though plain TS classifies write targets (`o.p = 2` classifies `p` as property). In editors with semantic highlighting, the one place a reactive variable is written is therefore the one occurrence left uncolored. A candidate fix is making the change-handler type in the internal types carry a real named property. Re-verify: request semantic tokens for `<let/count=1/><button onClick() { count = count + 1 }>`; the assignment LHS `count` has no token while the RHS does. Corroborated by inspecting the raw `getEncodedSemanticClassifications` output: the change object's property comes from a mapped type with no declarations, which TypeScript's classifier skips entirely, so no span is emitted at the rewritten access at all.

## Fix the change-data expression for destructured tag variables

`packages/language-tools/src/extractors/script/index.ts` › `#writeTag` | 2026-07-28 | impact:med | effort:low

For a destructured tag variable the emitted change data expression is `${renderVar}.return${objectPath}`, e.g. `__marko_internal_change__a = Marko._.change("a", __marko_internal_rendered_1.return.a)` — but `.return` is the returnTag object `{ value, valueChange }`, so `.a` lives under `.return.value`, not `.return`. TypeScript errors on the access, the diagnostic is dropped because its span is synthesized and maps to no source range, and the change object degrades to `any`, so destructured `<let>` variables accept any assignment without a type error. Re-verify: extract `<let/{a,b}={a:1,b:2}/>` plus a mutation of `a` and print `checker.typeToString` for `__marko_internal_change__a`; it is `any` today. Note the fix is not just redirecting the access to `.return.value.a`: `change` resolves `${sourceName}Change` (here `aChange`), which a plain property type never provides, so a naive redirect makes every destructured `<let>` readonly. Whoever fixes this must update `TagVarWritability` in `packages/language-server/src/service/script/index.ts` in the same pass, since its per-variable answer for destructured vars currently agrees with `doValidate` only by way of this bug's `any`.

## Emit `fallbackTemplate` tag names with `copy` so their diagnostics stay mapped

`packages/language-tools/src/extractors/script/index.ts` › `#writeTag` | 2026-07-28 | impact:low | effort:low

The `fallbackTemplate(<name>, import(...))` emission writes the tag name with `write` rather than `copy`, so any TypeScript diagnostic on that identifier is source-unmapped: the language server drops it silently (`convertDiag` cannot map the span) while the `mtc` CLI prints it without a location and still fails the run. The program-scope binding gate removed the two corpus instances that referenced undeclared names through this path, but the emission pattern remains for bound names that later break (eg a renamed import). Re-verify: emit a `fallbackTemplate` call whose first argument is an undeclared identifier and observe the diagnostic is absent from `doValidate` but printed location-less by `mtc`.
24 changes: 24 additions & 0 deletions agent-feedback/cleanup.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,27 @@
# Cleanup

Duplication, dead code, inconsistencies, refactor opportunities. Format and rules: [README.md](README.md).

## Delete the unused `__tests__/util/language-service.ts` helper

`packages/language-server/src/__tests__/util/language-service.ts` › (whole file) | 2026-07-27 | impact:low | effort:low

Nothing in the repo imports this file (`grep -rn "util/language-service" packages --include="*.ts"` returns zero hits); the fixture harness in `__tests__/index.test.ts` and the standalone tests import the service directly. Dead test scaffolding misleads new contributors into modeling work on it. Delete it; re-verify with the same grep before removal.

## Deduplicate the client/server watched-file globs (client lists `*.cts` twice, omits `*.cjs`)

`packages/vscode/src/index.ts` › `activate` | 2026-07-27 | impact:low | effort:low

The client-side `synchronize.fileEvents` glob repeats `*.cts` and is missing `*.cjs`, diverging from the server's `WATCHED_FILES_GLOB` in `packages/language-server/src/index.ts` which has both. On-disk changes to `.cjs` modules therefore don't invalidate caches for VS Code clients (the server-side dynamic registration covers other editors). Exporting the glob from the server package would give the two a single source of truth. Re-verify: `grep -o 'cts' packages/vscode/src/index.ts | wc -l` (currently 2 in one glob) and `grep -c 'cjs' packages/vscode/src/index.ts` (currently 0).

## Compute `resolveUrl` once per document link

`packages/language-server/src/service/marko/document-links.ts` › `findDocumentLinks` | 2026-07-27 | impact:low | effort:low

The `NodeType.Tag` branch calls `resolveUrl(read(attr.value.value).slice(1, -1), uri)` to test resolvability, then calls the identical expression a second time for the `target` property instead of reusing the `resolved` local it just bound, re-reading and re-resolving per link. Use `target: resolved`. Re-verify: both call sites are visible in the same `if` block in `findDocumentLinks`.

## Stop hardcoding `languageId: "css"` for scss/less virtual style documents

`packages/language-server/src/service/style/index.ts` › `processStyle` | 2026-07-27 | impact:low | effort:low

The virtual `TextDocument.create(uri, "css", ...)` always claims languageId `"css"` even when the extracted stylesheet is `.scss`/`.less` (the dialect is instead carried by which `vscode-css-languageservice` factory is chosen). Currently harmless because the language service ignores the id, but it is misleading and would break any future consumer that trusts `virtualDoc.languageId`. Pass the ext-appropriate id (`"scss"`/`"less"`), or add a comment stating why the id is intentionally unused. The virtual doc also deliberately reuses the source document's `uri`, and several downstream comparisons depend on that (e.g. the `symbol.location.uri === doc.uri` check in `findDocumentSymbols`), so the creation site deserves a comment about the uri as well. Re-verify: read the `TextDocument.create` call in `processStyle`.
12 changes: 12 additions & 0 deletions agent-feedback/perf.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
# Performance

Runtime speed and bundle size opportunities. Format and rules: [README.md](README.md).

## Plumb a cancellation token into the TS language-service host

`packages/language-server/src/service/script/index.ts` › `getTSProject` | 2026-07-28 | impact:med | effort:med

The language-service host provides no `getCancellationToken`, so TypeScript installs a no-op token and every TS call (`getSemanticDiagnostics`, `getEncodedSemanticClassifications`, completions) runs to completion even after the client cancels. Measured worst case for classification alone: ~128 ms uninterruptible at 137 KB of source (~380 ns per generated char); diagnostics are larger. The per-span cancellation check in the semantic-tokens mapping loop only covers ~3% of the work, and a pre-cancelled request still cost 127.7 of 128.0 ms. Wiring a per-request token through the host would make all TS-backed features responsive to cancellation. Re-verify: time any TS-backed request with an already-cancelled token; it currently costs the same as an uncancelled one.

## Narrow the classification span for semantic-token range requests

`packages/language-server/src/service/script/index.ts` › `getSemanticTokens` | 2026-07-28 | impact:low | effort:med

Range requests classify the entire generated file and filter afterwards. Per-version caching makes the second request of a version nearly free, but the first request, which paints the viewport, still pays full-document classification (~51 ms at 2,000 lines for a ~60-line viewport). Mapping the requested source range to a min/max generated span before calling `getEncodedSemanticClassifications` would cut that roughly proportionally, though the extractor hoists code, so generated ranges for a source range are non-contiguous and the span computation needs care. Re-verify: time a 60-line range request against a full request on a 2,000-line doc; they currently cost the same on a cold version.
Loading