feat: comptime evaluation - #3344
Open
DylanPiercey wants to merge 1 commit into
Open
Conversation
🦋 Changeset detectedLatest commit: bd59262 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
WalkthroughAdds Marko comptime syntax, compiler transformation, synchronous module evaluation, runtime value serialization, AST support, build output, and 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3344 +/- ##
==========================================
- Coverage 94.39% 94.18% -0.22%
==========================================
Files 389 395 +6
Lines 54223 56690 +2467
Branches 4450 4866 +416
==========================================
+ Hits 51186 53394 +2208
- Misses 3004 3259 +255
- Partials 33 37 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
DylanPiercey
force-pushed
the
claude/marko-comptime-design-le2g48
branch
from
July 10, 2026 01:50
511b658 to
9ccfe70
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
patches/@babel+types+7.29.7.patch (3)
174-180: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAlign
markoPlaceholder's default withMarkoPlaceholder.escape. The builder defaultsescapetofalse, butMarkoPlaceholder.escapedefaults totrue; omitting the argument changes escaping behavior and can emit unescaped output.🤖 Prompt for 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. In `@patches/`@babel+types+7.29.7.patch around lines 174 - 180, Update the markoPlaceholder builder’s default escape parameter to match MarkoPlaceholder.escape by changing its default from false to true, ensuring calls that omit the argument preserve escaped output.
910-913: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMatch
isMarkoagainstMARKO_TYPES
startsWith("Marko")accepts arbitraryMarko*strings likeMarkoUnknown, soisMarkocan return true for unsupported nodes. UseMARKO_TYPES.includes(node.type)instead.🤖 Prompt for 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. In `@patches/`@babel+types+7.29.7.patch around lines 910 - 913, Update isMarko to validate node.type against the supported MARKO_TYPES collection using MARKO_TYPES.includes(node.type), rather than checking whether the type starts with "Marko"; retain the existing null guard and opts shallowEqual behavior.
108-115: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlign
Program.paramsin the TypeScript declaration
node_modules/@babel/types/lib/index.d.tsstill makesProgram.paramsrequired asFunctionParameter[] | null, while the builder and field definition treat it as optional (params: undefined/optional: true). Make the declaration match that runtime shape and add a type-level regression test.🤖 Prompt for 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. In `@patches/`@babel+types+7.29.7.patch around lines 108 - 115, Update the Program-related TypeScript declaration in `@babel/types` so params is optional and matches the runtime builder’s params: undefined shape, rather than required FunctionParameter[] | null. Add a type-level regression test covering Program without params, using the existing Program builder/type-test conventions.
🧹 Nitpick comments (4)
packages/runtime-tags/src/translator/comptime/tags.ts (1)
7-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove helper declarations below the public exports.
The helpers currently precede all
Comptime*Tagexports. Convert them to function declarations and place them after the public API so the file follows the required top-down organization.As per coding guidelines: “Organize code top-down: put public API/exports first, then orchestration, then helpers, then low-level details last; use function-declaration hoisting to support that order.”
🤖 Prompt for 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. In `@packages/runtime-tags/src/translator/comptime/tags.ts` around lines 7 - 20, Move the helper declarations comptimeMissedGuard and comptimeTag below all public Comptime*Tag exports, converting them to function declarations so hoisting preserves behavior. Keep the public API and export definitions at the top, followed by these internal helpers.Source: Coding guidelines
packages/runtime-tags/src/translator/comptime/index.ts (1)
21-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPlace
comptimeTransformbefore private implementation details.Move the exported entry point above module augmentation, plan types, context types, and helper implementations.
As per coding guidelines, “put public API/exports first, then orchestration, then helpers, then low-level details last.”
🤖 Prompt for 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. In `@packages/runtime-tags/src/translator/comptime/index.ts` around lines 21 - 96, Move the exported comptimeTransform entry point to the top of the module, before the NodeExtra module augmentation, Plan and context type definitions, and private helper buildError. Keep the implementation unchanged while ordering the file as public API first, orchestration next, and private types/helpers afterward.Source: Coding guidelines
packages/runtime-tags/src/translator/comptime/evaluate.ts (1)
15-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove exported APIs above private implementation details.
Keep
ComptimeLoader,ComptimeModuleInput,createLoader, andevaluateComptimeModuletogether before cache types and helpers.As per coding guidelines, “put public API/exports first, then orchestration, then helpers, then low-level details last.”
Also applies to: 197-208
🤖 Prompt for 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. In `@packages/runtime-tags/src/translator/comptime/evaluate.ts` around lines 15 - 37, Reorganize the module so the public APIs ComptimeLoader, ComptimeModuleInput, createLoader, and evaluateComptimeModule appear together at the top, before private cache types, orchestration, and helper implementations. Preserve behavior while ordering the remaining private declarations from higher-level helpers to low-level details.Source: Coding guidelines
packages/runtime-tags/src/__tests__/fixtures/comptime-fold/test.ts (1)
3-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPlace the exported configuration before the helper.
Move
configaboveclick; the function declaration can remain below it because declarations are hoisted.Suggested reordering
-function click(container: Element) { - container.querySelector("button")!.click(); -} - export const config: TestConfig = { steps: [{}, click], }; + +function click(container: Element) { + container.querySelector("button")!.click(); +}As per coding guidelines,
**/*.{js,ts}files must put public API/exports first, then orchestration, then helpers, using function-declaration hoisting to support that order.🤖 Prompt for 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. In `@packages/runtime-tags/src/__tests__/fixtures/comptime-fold/test.ts` around lines 3 - 8, Reorder the declarations in the test fixture so the exported `config` constant appears before the `click` helper function. Keep `click` as a function declaration below `config`, relying on declaration hoisting while preserving the existing configuration behavior.Source: Coding guidelines
🤖 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 @.changeset/comptime-evaluation.md:
- Line 6: The release-note description should use the hyphenated term
“compile-time evaluation” instead of “compile time evaluation”; update that
phrase in the changeset while leaving the remaining content unchanged.
In `@comptime.md`:
- Line 690: The documentation incorrectly promises that error-recovery builds
skip comptime evaluation. Update the statements describing errorRecovery near
the “Preserve behavior is derived, not configured” section and line 739 to match
the current implementation, or change the implementation so error-recovery
compilation truly avoids evaluation before retaining that guarantee.
- Line 753: The documentation’s lexical module-loading guarantee conflicts with
user comptime access to wrapper parameters such as `$ctRequire`. Either prevent
these evaluator internals from being reachable in user comptime scope, or revise
the statement in comptime.md to qualify that data-derived loads through exposed
bindings are also possible.
- Around line 10-12: Reconcile the comptime implementation-status section with
the feature descriptions: inspect the documented behavior for <-define>,
<-return>, userland comptime tags, virtual modules, marko/comptime registration,
and cross-template storage, then mark each consistently as implemented or
planned. Update the opening status list so it does not contradict the later
sections, preserving only genuinely unimplemented items under the reserved or
not-yet-implemented entries.
In `@packages/runtime-tags/src/translator/comptime/evaluate.ts`:
- Around line 213-215: Replace the hardcoded identifiers in the comptime
evaluation wrapper with file-scoped, collision-free UIDs, ensuring the generated
`$ctRequire`, `$ctThunks`, and `$ctBindings` references consistently use those
allocated bindings throughout the wrapper generation logic.
- Around line 126-150: Update the module evaluation flow around the cached
module created in the `moduleCache!.set` block and executed by `wrapper`: catch
evaluation errors, remove `resolved` from `moduleCache` before rethrowing, and
preserve successful caching and return behavior.
In `@packages/runtime-tags/src/translator/comptime/index.ts`:
- Around line 1014-1025: Update the range handling in the “range” case to reject
a zero step, then compare loop values directly with edge: use <= or >= for
inclusive ranges and < or > for exclusive ranges, preserving direction based on
step sign. Remove the edge adjustment by ±1 so fractional steps such as .25 work
correctly.
- Around line 522-580: collectRenameGroups must also collect bindings declared
by runtime scriptlets within the <-for> body, not only MarkoTag vars and
parameters. Extend the traversal to identify non-comptime runtime scriptlet
declarations, obtain their bindings and scope, and pass them through the
existing collect logic so declarations, references, and assignments receive
per-copy rename groups.
In `@packages/runtime-tags/src/translator/comptime/serialize.ts`:
- Around line 319-334: In the object serialization logic within write, emit the
__proto__ key as a computed string-literal property (["__proto__"]) rather than
a regular object property, while preserving the existing identifier handling for
other valid keys.
- Around line 187-203: Update the serialization logic around the ref creation
and hoisting path in the relevant serializer function so the parent’s
declaration index is assigned after writeUnserializable/writeObject has
serialized nested dependencies, not before. Preserve the ref registration needed
for cycle detection, then record the correct post-dependency position before
later reuse can hoist the parent, ensuring its declaration is emitted after all
referenced declarations.
---
Outside diff comments:
In `@patches/`@babel+types+7.29.7.patch:
- Around line 174-180: Update the markoPlaceholder builder’s default escape
parameter to match MarkoPlaceholder.escape by changing its default from false to
true, ensuring calls that omit the argument preserve escaped output.
- Around line 910-913: Update isMarko to validate node.type against the
supported MARKO_TYPES collection using MARKO_TYPES.includes(node.type), rather
than checking whether the type starts with "Marko"; retain the existing null
guard and opts shallowEqual behavior.
- Around line 108-115: Update the Program-related TypeScript declaration in
`@babel/types` so params is optional and matches the runtime builder’s params:
undefined shape, rather than required FunctionParameter[] | null. Add a
type-level regression test covering Program without params, using the existing
Program builder/type-test conventions.
---
Nitpick comments:
In `@packages/runtime-tags/src/__tests__/fixtures/comptime-fold/test.ts`:
- Around line 3-8: Reorder the declarations in the test fixture so the exported
`config` constant appears before the `click` helper function. Keep `click` as a
function declaration below `config`, relying on declaration hoisting while
preserving the existing configuration behavior.
In `@packages/runtime-tags/src/translator/comptime/evaluate.ts`:
- Around line 15-37: Reorganize the module so the public APIs ComptimeLoader,
ComptimeModuleInput, createLoader, and evaluateComptimeModule appear together at
the top, before private cache types, orchestration, and helper implementations.
Preserve behavior while ordering the remaining private declarations from
higher-level helpers to low-level details.
In `@packages/runtime-tags/src/translator/comptime/index.ts`:
- Around line 21-96: Move the exported comptimeTransform entry point to the top
of the module, before the NodeExtra module augmentation, Plan and context type
definitions, and private helper buildError. Keep the implementation unchanged
while ordering the file as public API first, orchestration next, and private
types/helpers afterward.
In `@packages/runtime-tags/src/translator/comptime/tags.ts`:
- Around line 7-20: Move the helper declarations comptimeMissedGuard and
comptimeTag below all public Comptime*Tag exports, converting them to function
declarations so hoisting preserves behavior. Keep the public API and export
definitions at the top, followed by these internal helpers.
🪄 Autofix (Beta)
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: 66a3edbf-b110-4fed-ad9c-67cf0f30ab83
⛔ Files ignored due to path filters (109)
packages/runtime-tags/src/__tests__/fixtures/comptime-at-tags/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-at-tags/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-at-tags/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-at-tags/__snapshots__/render.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-at-tags/__snapshots__/render.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-at-tags/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-at-tags/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-fold/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-fold/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-fold/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-fold/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-fold/__snapshots__/render.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-fold/__snapshots__/render.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-fold/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-fold/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for-interactive/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for-interactive/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for-interactive/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for-interactive/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for-interactive/__snapshots__/render.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for-interactive/__snapshots__/render.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for-interactive/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for-interactive/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for/__snapshots__/render.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for/__snapshots__/render.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-for/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-if/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-if/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-if/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-if/__snapshots__/render-csr.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-if/__snapshots__/render-ssr.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-if/__snapshots__/render-ssr.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-if/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-if/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-import/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-import/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-import/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-import/__snapshots__/render-csr.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-import/__snapshots__/render-ssr.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-import/__snapshots__/render-ssr.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-import/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-import/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-serialize/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-serialize/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-serialize/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-serialize/__snapshots__/render-csr.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-serialize/__snapshots__/render-ssr.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-serialize/__snapshots__/render-ssr.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-serialize/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-serialize/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-statement/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-statement/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-statement/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-statement/__snapshots__/render.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-statement/__snapshots__/render.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-statement/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/comptime-statement/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-async/__snapshots__/error-compile-dom.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-async/__snapshots__/error-compile-dom.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-async/__snapshots__/error-compile-html.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-async/__snapshots__/error-compile-html.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-define/__snapshots__/error-compile-dom.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-define/__snapshots__/error-compile-dom.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-define/__snapshots__/error-compile-html.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-define/__snapshots__/error-compile-html.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-export/__snapshots__/error-compile-dom.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-export/__snapshots__/error-compile-dom.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-export/__snapshots__/error-compile-html.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-export/__snapshots__/error-compile-html.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-for-by/__snapshots__/error-compile-dom.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-for-by/__snapshots__/error-compile-dom.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-for-by/__snapshots__/error-compile-html.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-for-by/__snapshots__/error-compile-html.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-input/__snapshots__/error-compile-dom.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-input/__snapshots__/error-compile-dom.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-input/__snapshots__/error-compile-html.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-input/__snapshots__/error-compile-html.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-marko-import/__snapshots__/error-compile-dom.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-marko-import/__snapshots__/error-compile-dom.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-marko-import/__snapshots__/error-compile-html.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-marko-import/__snapshots__/error-compile-html.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-max-iterations/__snapshots__/error-compile-dom.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-max-iterations/__snapshots__/error-compile-dom.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-max-iterations/__snapshots__/error-compile-html.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-max-iterations/__snapshots__/error-compile-html.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-plain-import/__snapshots__/error-compile-dom.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-plain-import/__snapshots__/error-compile-dom.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-plain-import/__snapshots__/error-compile-html.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-plain-import/__snapshots__/error-compile-html.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-return/__snapshots__/error-compile-dom.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-return/__snapshots__/error-compile-dom.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-return/__snapshots__/error-compile-html.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-return/__snapshots__/error-compile-html.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-runtime-binding/__snapshots__/error-compile-dom.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-runtime-binding/__snapshots__/error-compile-dom.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-runtime-binding/__snapshots__/error-compile-html.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-runtime-binding/__snapshots__/error-compile-html.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-serialize/__snapshots__/error-compile-dom.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-serialize/__snapshots__/error-compile-dom.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-serialize/__snapshots__/error-compile-html.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-serialize/__snapshots__/error-compile-html.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-userland/__snapshots__/error-compile-dom.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-userland/__snapshots__/error-compile-dom.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-userland/__snapshots__/error-compile-html.debug.txtis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/error-comptime-userland/__snapshots__/error-compile-html.txtis excluded by!**/__snapshots__/**and included by**
📒 Files selected for processing (70)
.changeset/comptime-evaluation.mdagent-feedback/bugs.mdagent-feedback/cleanup.mdagent-feedback/dx.mdagent-feedback/perf.mdcomptime.mdcspell.jsonpackages/runtime-class/test/taglib-lookup/fixtures/getTagsSorted/expected.jsonpackages/runtime-tags/src/__tests__/fixtures/comptime-at-tags/sections.jsonpackages/runtime-tags/src/__tests__/fixtures/comptime-at-tags/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/comptime-at-tags/tags/my-tabs/index.markopackages/runtime-tags/src/__tests__/fixtures/comptime-at-tags/template.markopackages/runtime-tags/src/__tests__/fixtures/comptime-fold/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/comptime-fold/template.markopackages/runtime-tags/src/__tests__/fixtures/comptime-fold/test.tspackages/runtime-tags/src/__tests__/fixtures/comptime-for-interactive/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/comptime-for-interactive/template.markopackages/runtime-tags/src/__tests__/fixtures/comptime-for-interactive/test.tspackages/runtime-tags/src/__tests__/fixtures/comptime-for/plans.jsonpackages/runtime-tags/src/__tests__/fixtures/comptime-for/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/comptime-for/template.markopackages/runtime-tags/src/__tests__/fixtures/comptime-if/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/comptime-if/template.markopackages/runtime-tags/src/__tests__/fixtures/comptime-if/test.tspackages/runtime-tags/src/__tests__/fixtures/comptime-import/catalog.jsonpackages/runtime-tags/src/__tests__/fixtures/comptime-import/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/comptime-import/template.markopackages/runtime-tags/src/__tests__/fixtures/comptime-import/test.tspackages/runtime-tags/src/__tests__/fixtures/comptime-import/util/money.tspackages/runtime-tags/src/__tests__/fixtures/comptime-import/util/shout.tspackages/runtime-tags/src/__tests__/fixtures/comptime-serialize/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/comptime-serialize/template.markopackages/runtime-tags/src/__tests__/fixtures/comptime-serialize/test.tspackages/runtime-tags/src/__tests__/fixtures/comptime-statement/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/comptime-statement/template.markopackages/runtime-tags/src/__tests__/fixtures/comptime-statement/util/repeat.tspackages/runtime-tags/src/__tests__/fixtures/error-comptime-async/template.markopackages/runtime-tags/src/__tests__/fixtures/error-comptime-async/test.tspackages/runtime-tags/src/__tests__/fixtures/error-comptime-define/template.markopackages/runtime-tags/src/__tests__/fixtures/error-comptime-define/test.tspackages/runtime-tags/src/__tests__/fixtures/error-comptime-export/template.markopackages/runtime-tags/src/__tests__/fixtures/error-comptime-export/test.tspackages/runtime-tags/src/__tests__/fixtures/error-comptime-for-by/template.markopackages/runtime-tags/src/__tests__/fixtures/error-comptime-for-by/test.tspackages/runtime-tags/src/__tests__/fixtures/error-comptime-input/template.markopackages/runtime-tags/src/__tests__/fixtures/error-comptime-input/test.tspackages/runtime-tags/src/__tests__/fixtures/error-comptime-marko-import/other.markopackages/runtime-tags/src/__tests__/fixtures/error-comptime-marko-import/template.markopackages/runtime-tags/src/__tests__/fixtures/error-comptime-marko-import/test.tspackages/runtime-tags/src/__tests__/fixtures/error-comptime-max-iterations/template.markopackages/runtime-tags/src/__tests__/fixtures/error-comptime-max-iterations/test.tspackages/runtime-tags/src/__tests__/fixtures/error-comptime-plain-import/limit.tspackages/runtime-tags/src/__tests__/fixtures/error-comptime-plain-import/template.markopackages/runtime-tags/src/__tests__/fixtures/error-comptime-plain-import/test.tspackages/runtime-tags/src/__tests__/fixtures/error-comptime-return/template.markopackages/runtime-tags/src/__tests__/fixtures/error-comptime-return/test.tspackages/runtime-tags/src/__tests__/fixtures/error-comptime-runtime-binding/template.markopackages/runtime-tags/src/__tests__/fixtures/error-comptime-runtime-binding/test.tspackages/runtime-tags/src/__tests__/fixtures/error-comptime-serialize/template.markopackages/runtime-tags/src/__tests__/fixtures/error-comptime-serialize/test.tspackages/runtime-tags/src/__tests__/fixtures/error-comptime-userland/template.markopackages/runtime-tags/src/__tests__/fixtures/error-comptime-userland/test.tspackages/runtime-tags/src/translator/comptime/evaluate.tspackages/runtime-tags/src/translator/comptime/index.tspackages/runtime-tags/src/translator/comptime/serialize.tspackages/runtime-tags/src/translator/comptime/tags.tspackages/runtime-tags/src/translator/core/comptime.tspackages/runtime-tags/src/translator/core/index.tspackages/runtime-tags/src/translator/visitors/program/index.tspatches/@babel+types+7.29.7.patch
| "@marko/compiler": minor | ||
| --- | ||
|
|
||
| Add compile time evaluation: `comptime` statements/imports and the `<-if>`, `<-for>`, `<-const>`, `<-log>`, and `<-debug>` tags run while compiling and leave only their expanded output. Values reach runtime through a resume-style AST serializer, and primitive reads fold into eagerly escaped static output. `<-define>`, `<-return>`, and userland comptime tags are reserved. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the hyphenated form compile-time.
This is a public release-note description; update “compile time evaluation” to “compile-time evaluation.”
🧰 Tools
🪛 LanguageTool
[grammar] ~6-~6: Use a hyphen to join words.
Context: ...@marko/compiler": minor --- Add compile time evaluation: comptime statements/i...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for 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.
In @.changeset/comptime-evaluation.md at line 6, The release-note description
should use the hyphenated term “compile-time evaluation” instead of “compile
time evaluation”; update that phrase in the changeset while leaving the
remaining content unchanged.
Source: Linters/SAST tools
Comment on lines
+126
to
+150
| // Cached before evaluation so require cycles see partial exports. | ||
| moduleCache!.set( | ||
| resolved, | ||
| (mod = { | ||
| get exports() { | ||
| return module.exports; | ||
| }, | ||
| get namespace() { | ||
| return (namespace ??= toNamespace(module.exports)); | ||
| }, | ||
| }), | ||
| ); | ||
|
|
||
| const wrapper = vm.runInThisContext( | ||
| `(function(exports, require, module, __filename, __dirname){${code}\n})`, | ||
| { filename: resolved }, | ||
| ); | ||
| wrapper( | ||
| module.exports, | ||
| (spec: string) => load(spec, resolved).exports, | ||
| module, | ||
| resolved, | ||
| path.dirname(resolved), | ||
| ); | ||
| return mod; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Evict modules whose evaluation throws.
The module enters the shared cache before execution, but an exception leaves partial exports cached for subsequent templates.
Proposed fix
- wrapper(
- module.exports,
- (spec: string) => load(spec, resolved).exports,
- module,
- resolved,
- path.dirname(resolved),
- );
+ try {
+ wrapper(
+ module.exports,
+ (spec: string) => load(spec, resolved).exports,
+ module,
+ resolved,
+ path.dirname(resolved),
+ );
+ } catch (err) {
+ moduleCache!.delete(resolved);
+ throw err;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Cached before evaluation so require cycles see partial exports. | |
| moduleCache!.set( | |
| resolved, | |
| (mod = { | |
| get exports() { | |
| return module.exports; | |
| }, | |
| get namespace() { | |
| return (namespace ??= toNamespace(module.exports)); | |
| }, | |
| }), | |
| ); | |
| const wrapper = vm.runInThisContext( | |
| `(function(exports, require, module, __filename, __dirname){${code}\n})`, | |
| { filename: resolved }, | |
| ); | |
| wrapper( | |
| module.exports, | |
| (spec: string) => load(spec, resolved).exports, | |
| module, | |
| resolved, | |
| path.dirname(resolved), | |
| ); | |
| return mod; | |
| // Cached before evaluation so require cycles see partial exports. | |
| moduleCache!.set( | |
| resolved, | |
| (mod = { | |
| get exports() { | |
| return module.exports; | |
| }, | |
| get namespace() { | |
| return (namespace ??= toNamespace(module.exports)); | |
| }, | |
| }), | |
| ); | |
| const wrapper = vm.runInThisContext( | |
| `(function(exports, require, module, __filename, __dirname){${code}\n})`, | |
| { filename: resolved }, | |
| ); | |
| try { | |
| wrapper( | |
| module.exports, | |
| (spec: string) => load(spec, resolved).exports, | |
| module, | |
| resolved, | |
| path.dirname(resolved), | |
| ); | |
| } catch (err) { | |
| moduleCache!.delete(resolved); | |
| throw err; | |
| } | |
| return mod; |
🤖 Prompt for 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.
In `@packages/runtime-tags/src/translator/comptime/evaluate.ts` around lines 126 -
150, Update the module evaluation flow around the cached module created in the
`moduleCache!.set` block and executed by `wrapper`: catch evaluation errors,
remove `resolved` from `moduleCache` before rethrowing, and preserve successful
caching and return behavior.
Comment on lines
+213
to
+215
| const requireId = t.identifier("$ctRequire"); | ||
| const thunksId = t.identifier("$ctThunks"); | ||
| const bindingsId = t.identifier("$ctBindings"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Generate collision-free wrapper bindings.
The hardcoded $ctRequire, $ctThunks, and $ctBindings names can collide with valid comptime declarations or import locals, making the generated factory invalid. Allocate file-scoped UIDs instead.
🤖 Prompt for 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.
In `@packages/runtime-tags/src/translator/comptime/evaluate.ts` around lines 213 -
215, Replace the hardcoded identifiers in the comptime evaluation wrapper with
file-scoped, collision-free UIDs, ensuring the generated `$ctRequire`,
`$ctThunks`, and `$ctBindings` references consistently use those allocated
bindings throughout the wrapper generation logic.
Comment on lines
+522
to
+580
| /** Stamps runtime bindings declared in a `<-for>` body so each unrolled | ||
| * copy can alpha-rename them (avoiding duplicate declarations). */ | ||
| function collectRenameGroups(ctx: PassContext, tag: t.NodePath<t.MarkoTag>) { | ||
| const groups: { group: number; name: string }[] = []; | ||
|
|
||
| const collect = ( | ||
| identifiers: Record<string, t.Identifier | t.Identifier[]>, | ||
| scope: t.Scope, | ||
| ) => { | ||
| for (const name of Object.keys(identifiers)) { | ||
| const binding = scope.getBinding(name); | ||
| if (!binding) continue; | ||
|
|
||
| const group = ctx.renameGroupCount++; | ||
| groups.push({ group, name }); | ||
| stampRename(binding.identifier, group); | ||
| for (const ref of binding.referencePaths) { | ||
| if (ref.isIdentifier()) { | ||
| if (!isWithin(ref, tag)) { | ||
| throw ref.buildCodeFrameError( | ||
| `\`${name}\` is declared inside a \`<-for>\` body (one binding per unrolled copy) and cannot be referenced from outside it.`, | ||
| ); | ||
| } | ||
| stampRename(ref.node, group); | ||
| } | ||
| } | ||
| for (const violation of binding.constantViolations) { | ||
| const target = violation.isAssignmentExpression() | ||
| ? violation.node.left | ||
| : violation.isUpdateExpression() | ||
| ? violation.node.argument | ||
| : undefined; | ||
| if (t.isIdentifier(target, { name })) { | ||
| if (!isWithin(violation, tag)) { | ||
| throw violation.buildCodeFrameError( | ||
| `\`${name}\` is declared inside a \`<-for>\` body (one binding per unrolled copy) and cannot be assigned from outside it.`, | ||
| ); | ||
| } | ||
| stampRename(target, group); | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| tag.get("body").traverse({ | ||
| MarkoTag(child) { | ||
| // Comptime tags declare comptime bindings (handled through slots). | ||
| if (isComptimeTagNode(child.node)) return; | ||
| if (child.node.var) { | ||
| collect(t.getBindingIdentifiers(child.node.var as any), child.scope); | ||
| } | ||
| for (const param of child.node.body.params) { | ||
| collect(t.getBindingIdentifiers(param as any), child.get("body").scope); | ||
| } | ||
| }, | ||
| }); | ||
|
|
||
| return groups; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include runtime scriptlet declarations in per-copy renaming.
Only MarkoTag bindings are collected. Runtime scriptlet declarations inside <-for> are cloned with identical names for every iteration, producing duplicate declarations and unrenamed references.
🤖 Prompt for 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.
In `@packages/runtime-tags/src/translator/comptime/index.ts` around lines 522 -
580, collectRenameGroups must also collect bindings declared by runtime
scriptlets within the <-for> body, not only MarkoTag vars and parameters. Extend
the traversal to identify non-comptime runtime scriptlet declarations, obtain
their bindings and scope, and pass them through the existing collect logic so
declarations, references, and assignments receive per-copy rename groups.
Comment on lines
+1014
to
+1025
| case "range": { | ||
| const [from, edge, step] = callThunk( | ||
| ctx, | ||
| node, | ||
| plan.rangeThunk!, | ||
| thunkArgs(plan, env), | ||
| ) as [number, number, number]; | ||
| const to = plan.inclusive ? edge : edge - (step > 0 ? 1 : -1); | ||
| for (let i = from; step > 0 ? i <= to : i >= to; i += step) { | ||
| iteration([i]); | ||
| } | ||
| break; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Compare exclusive ranges directly against the edge.
Subtracting or adding 1 only models integer ranges. For example, from=0 until=1 step=.25 emits only 0. Use </> for until, <=/>= for to, and reject a zero step.
🤖 Prompt for 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.
In `@packages/runtime-tags/src/translator/comptime/index.ts` around lines 1014 -
1025, Update the range handling in the “range” case to reject a zero step, then
compare loop values directly with edge: use <= or >= for inclusive ranges and <
or > for exclusive ranges, preserving direction based on step sign. Remove the
edge adjustment by ±1 so fractional steps such as .25 work correctly.
Comment on lines
+187
to
+203
| ref = { declIndex: ctx.decls.length, building: true }; | ||
| ctx.refs.set(value, ref); | ||
| const node = | ||
| typeof value === "function" | ||
| ? writeUnserializable(ctx, value, frame) | ||
| : writeObject(ctx, value, ref, frame); | ||
| ref.building = false; | ||
|
|
||
| if (ref.id) { | ||
| // A cycle claimed an id while this value was being built: hoist it now. | ||
| ctx.decls.push(constDecl(ref.id, node)); | ||
| ref.declIndex = ctx.decls.length - 1; | ||
| return t.identifier(ref.id); | ||
| } | ||
|
|
||
| ref.node = node; | ||
| return node; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Record the hoist position after serializing dependencies.
declIndex is captured before nested shared values add declarations. If the parent is later reused, it is inserted before those dependencies and references them in the TDZ.
Proposed fix
const node =
typeof value === "function"
? writeUnserializable(ctx, value, frame)
: writeObject(ctx, value, ref, frame);
ref.building = false;
+ ref.declIndex = ctx.decls.length;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ref = { declIndex: ctx.decls.length, building: true }; | |
| ctx.refs.set(value, ref); | |
| const node = | |
| typeof value === "function" | |
| ? writeUnserializable(ctx, value, frame) | |
| : writeObject(ctx, value, ref, frame); | |
| ref.building = false; | |
| if (ref.id) { | |
| // A cycle claimed an id while this value was being built: hoist it now. | |
| ctx.decls.push(constDecl(ref.id, node)); | |
| ref.declIndex = ctx.decls.length - 1; | |
| return t.identifier(ref.id); | |
| } | |
| ref.node = node; | |
| return node; | |
| ref = { declIndex: ctx.decls.length, building: true }; | |
| ctx.refs.set(value, ref); | |
| const node = | |
| typeof value === "function" | |
| ? writeUnserializable(ctx, value, frame) | |
| : writeObject(ctx, value, ref, frame); | |
| ref.building = false; | |
| ref.declIndex = ctx.decls.length; | |
| if (ref.id) { | |
| // A cycle claimed an id while this value was being built: hoist it now. | |
| ctx.decls.push(constDecl(ref.id, node)); | |
| ref.declIndex = ctx.decls.length - 1; | |
| return t.identifier(ref.id); | |
| } | |
| ref.node = node; | |
| return node; |
🤖 Prompt for 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.
In `@packages/runtime-tags/src/translator/comptime/serialize.ts` around lines 187
- 203, Update the serialization logic around the ref creation and hoisting path
in the relevant serializer function so the parent’s declaration index is
assigned after writeUnserializable/writeObject has serialized nested
dependencies, not before. Preserve the ref registration needed for cycle
detection, then record the correct post-dependency position before later reuse
can hoist the parent, ensuring its declaration is emitted after all referenced
declarations.
Comment on lines
+319
to
+334
| const proto = Object.getPrototypeOf(value); | ||
| if (proto === Object.prototype || proto === null) { | ||
| const props: t.ObjectProperty[] = []; | ||
| for (const key of Object.keys(value)) { | ||
| props.push( | ||
| t.objectProperty( | ||
| isValidKey(key) ? t.identifier(key) : t.stringLiteral(key), | ||
| write(ctx, (value as Record<string, unknown>)[key], { | ||
| parent: frame, | ||
| container: ref, | ||
| accessor: key, | ||
| }), | ||
| ), | ||
| ); | ||
| } | ||
| return t.objectExpression(props); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Emit __proto__ as a computed property.
Generating __proto__: value changes the new object's prototype instead of restoring an own data property. Emit ["__proto__"] for this key.
🤖 Prompt for 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.
In `@packages/runtime-tags/src/translator/comptime/serialize.ts` around lines 319
- 334, In the object serialization logic within write, emit the __proto__ key as
a computed string-literal property (["__proto__"]) rather than a regular object
property, while preserving the existing identifier handling for other valid
keys.
DylanPiercey
force-pushed
the
claude/marko-comptime-design-le2g48
branch
2 times, most recently
from
July 18, 2026 01:54
3d5f5b8 to
547a811
Compare
`comptime` statements/imports and `-` prefixed tags (`<-if>`, `<-for>`, `<-const>`, `<-log>`, `<-debug>`) evaluate while compiling and leave only their expanded output. Values reach runtime through a resume-style AST serializer; primitive reads fold into eagerly escaped static output. Design doc: comptime.md.
DylanPiercey
force-pushed
the
claude/marko-comptime-design-le2g48
branch
from
July 18, 2026 02:41
547a811 to
bd59262
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/runtime-tags/src/__tests__/comptime-compile.test.ts (1)
9-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRestore the required top-down module layout.
packages/runtime-tags/src/__tests__/comptime-compile.test.ts#L9-L35: place thedescribeorchestration before setup helpers and makecompileTemplatea hoisted function declaration.packages/runtime-tags/src/__tests__/comptime-compile/shared.ts#L1-L7: move global-state initialization into a helper below the exported APIs.As per coding guidelines,
**/*.{js,ts,tsx,marko}files must organize public APIs and exports first, orchestration next, then helpers, and use function-declaration hoisting.🤖 Prompt for 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. In `@packages/runtime-tags/src/__tests__/comptime-compile.test.ts` around lines 9 - 35, Reorder packages/runtime-tags/src/__tests__/comptime-compile.test.ts so the describe orchestration appears before setup helpers, and convert compileTemplate to a hoisted function declaration while preserving its behavior. In packages/runtime-tags/src/__tests__/comptime-compile/shared.ts, move global-state initialization into a helper located below the exported APIs. Follow the required public APIs/exports, orchestration, then helpers layout in both files.Source: Coding guidelines
🤖 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 @.changeset/comptime-evaluation.md:
- Line 6: Update the comptime changeset to remove or revise claims that userland
“-” tags, <-define>, and <-return> are implemented, matching the supported
surface and objectives that reserve these constructs as not implemented. Keep
the documentation accurate without changing the descriptions of features that
are actually available.
In `@comptime.md`:
- Line 104: Update the “Real JavaScript, really executed” section to qualify the
“If it runs in Node, it runs in comptime” claim as applying only to synchronous
JavaScript supported by the evaluator. Preserve the statements about real
compiler globals, imports, and semantics while explicitly excluding pending
promises and deferred top-level await.
- Line 805: Update the “Runtime cost is zero by construction” statement in the
comptime principles documentation to remove the zero-size and zero-cost
guarantee. Limit the claim to comptime introducing no new runtime machinery, and
state that generated output retains the normal runtime bytes and work of its
unrolled bodies, serialized values, and virtual modules/functions.
- Around line 278-279: Update the “Real module identity” section to describe
comptime-imported module instances as scoped to the active build/cache lifetime
rather than the compiler process. Align the statement with the later cache
contract by documenting that invalidation clears the module state, and avoid
claiming process-wide persistence across revisions.
---
Nitpick comments:
In `@packages/runtime-tags/src/__tests__/comptime-compile.test.ts`:
- Around line 9-35: Reorder
packages/runtime-tags/src/__tests__/comptime-compile.test.ts so the describe
orchestration appears before setup helpers, and convert compileTemplate to a
hoisted function declaration while preserving its behavior. In
packages/runtime-tags/src/__tests__/comptime-compile/shared.ts, move
global-state initialization into a helper located below the exported APIs.
Follow the required public APIs/exports, orchestration, then helpers layout in
both files.
🪄 Autofix (Beta)
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: 19867112-785f-4bd9-911b-4c206b774627
📒 Files selected for processing (10)
.changeset/comptime-evaluation.mdagent-feedback/bugs.mdagent-feedback/dx.mdagent-feedback/perf.mdcomptime.mdpackages/runtime-class/test/taglib-lookup/fixtures/getTagsSorted/expected.jsonpackages/runtime-tags/scripts/bundle.mtspackages/runtime-tags/src/__tests__/comptime-compile.test.tspackages/runtime-tags/src/__tests__/comptime-compile/shared.tspackages/runtime-tags/src/__tests__/comptime-inert.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/runtime-class/test/taglib-lookup/fixtures/getTagsSorted/expected.json
- agent-feedback/dx.md
- agent-feedback/bugs.md
| "@marko/compiler": minor | ||
| --- | ||
|
|
||
| Add compile time evaluation: `comptime` statements/imports and the `<-if>`, `<-for>`, `<-const>`, `<-log>`, and `<-debug>` tags run while compiling and leave only their expanded output. Any userland tag invoked with the `-` prefix macro-expands its template into the caller with `input` known at compile time (content stays live and interactive), `<-define>` declares in-file snippets, and `<-return>` gives comptime invocations a result value. Values reach runtime through a resume-style AST serializer; primitive reads fold into eagerly escaped static output, non-primitive member reads serialize just the accessed subtree, and comptime-born functions cross as capture-applied factories in a content-addressed virtual module (inline without a bundler hook). The new `marko/comptime` module ships `inert`/`isInert`, an airlock that snapshots untrusted data into frozen null-prototype trees safe to hand to comptime evaluation. Evaluation is bounded by fixed unroll, expansion-depth, and wall-clock guards, errors chain the user throw as `cause` with statement-anchored code frames and expansion traces, `<-log>` surfaces as compiler diagnostics, compile results carry `meta.comptime`, and `errorRecovery` compiles (editor tooling) preserve comptime constructs without executing them. `$ct`-prefixed names are reserved in comptime code. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Document the implemented comptime surface accurately.
This release note says userland - tags, <-define>, and <-return> are implemented, but the PR objectives state they are reserved and report as not implemented. Update the changeset to avoid advertising unsupported behavior.
Suggested correction
-Any userland tag invoked with the `-` prefix macro-expands its template into the caller with `input` known at compile time (content stays live and interactive), `<-define>` declares in-file snippets, and `<-return>` gives comptime invocations a result value.
+`<-define>`, `<-return>`, and userland comptime tags are reserved and report as not implemented.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Add compile time evaluation: `comptime` statements/imports and the `<-if>`, `<-for>`, `<-const>`, `<-log>`, and `<-debug>` tags run while compiling and leave only their expanded output. Any userland tag invoked with the `-` prefix macro-expands its template into the caller with `input` known at compile time (content stays live and interactive), `<-define>` declares in-file snippets, and `<-return>` gives comptime invocations a result value. Values reach runtime through a resume-style AST serializer; primitive reads fold into eagerly escaped static output, non-primitive member reads serialize just the accessed subtree, and comptime-born functions cross as capture-applied factories in a content-addressed virtual module (inline without a bundler hook). The new `marko/comptime` module ships `inert`/`isInert`, an airlock that snapshots untrusted data into frozen null-prototype trees safe to hand to comptime evaluation. Evaluation is bounded by fixed unroll, expansion-depth, and wall-clock guards, errors chain the user throw as `cause` with statement-anchored code frames and expansion traces, `<-log>` surfaces as compiler diagnostics, compile results carry `meta.comptime`, and `errorRecovery` compiles (editor tooling) preserve comptime constructs without executing them. `$ct`-prefixed names are reserved in comptime code. | |
| Add compile time evaluation: `comptime` statements/imports and the `<-if>`, `<-for>`, `<-const>`, `<-log>`, and `<-debug>` tags run while compiling and leave only their expanded output. `<-define>`, `<-return>`, and userland comptime tags are reserved and report as not implemented. Values reach runtime through a resume-style AST serializer; primitive reads fold into eagerly escaped static output, non-primitive member reads serialize just the accessed subtree, and comptime-born functions cross as capture-applied factories in a content-addressed virtual module (inline without a bundler hook). The new `marko/comptime` module ships `inert`/`isInert`, an airlock that snapshots untrusted data into frozen null-prototype trees safe to hand to comptime evaluation. Evaluation is bounded by fixed unroll, expansion-depth, and wall-clock guards, errors chain the user throw as `cause` with statement-anchored code frames and expansion traces, `<-log>` surfaces as compiler diagnostics, compile results carry `meta.comptime`, and `errorRecovery` compiles (editor tooling) preserve comptime constructs without executing them. `$ct`-prefixed names are reserved in comptime code. |
🧰 Tools
🪛 LanguageTool
[grammar] ~6-~6: Use a hyphen to join words.
Context: ...@marko/compiler": minor --- Add compile time evaluation: comptime statements/i...
(QB_NEW_EN_HYPHEN)
[grammar] ~6-~6: Ensure spelling is correct
Context: ...all-clock guards, errors chain the user throw as cause with statement-anchored code...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for 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.
In @.changeset/comptime-evaluation.md at line 6, Update the comptime changeset
to remove or revise claims that userland “-” tags, <-define>, and <-return> are
implemented, matching the supported surface and objectives that reserve these
constructs as not implemented. Keep the documentation accurate without changing
the descriptions of features that are actually available.
|
|
||
| 2. **The time ladder, with resume-style handoffs between rungs.** Marko statements already form a ladder of "when does this run": `static` (module load, both environments) and `server` / `client` (module load, one environment), then per-render code, then interactive updates. Comptime adds the rung above: compile time. Every comptime construct lowers onto the next rung down — `comptime const` becomes `static const`, `<-if>` becomes presence or absence, `<-for>` becomes repetition. Nothing lowers "sideways" into new runtime machinery. And Marko already ships one boundary crossing on this ladder — the server→client resume handoff, powered by `html/serializer.ts` — so comptime→runtime is specified as the same kind of handoff one rung earlier, sharing that serializer's architecture (see [Crossing the boundary](#crossing-the-boundary-the-comptime-handoff)). | ||
|
|
||
| 3. **Real JavaScript, really executed.** Comptime expressions are evaluated by running them as JavaScript in the compiler's process, with the compiler's globals, real imports, and real semantics. There is no interpreter subset, no "constant expression" dialect, no surprise at `"a".repeat(5)` working but `[...Array(5)]` not. If it runs in Node, it runs in comptime. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scope the “if it runs in Node” claim to supported synchronous evaluation.
The v1 contract rejects pending promises and defers top-level await, so not everything that runs in Node runs in comptime. Reword this to limit the claim to synchronous JavaScript supported by the evaluator.
🤖 Prompt for 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.
In `@comptime.md` at line 104, Update the “Real JavaScript, really executed”
section to qualify the “If it runs in Node, it runs in comptime” claim as
applying only to synchronous JavaScript supported by the evaluator. Preserve the
statements about real compiler globals, imports, and semantics while explicitly
excluding pending promises and deferred top-level await.
Comment on lines
+278
to
+279
| - **Real module identity.** `import.meta.url` is the template's file URL. A comptime-imported module is instantiated once per compiler process (normal ESM caching), so two templates that `comptime import` the same CMS SDK share its state — the same rule as any two modules in an app. | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Describe module identity as build/cache-scoped, not process-scoped.
This says comptime-imported modules are instantiated once per compiler process, but the later cache contract is build-scoped and invalidation clears that state. A process-level module cache could retain stale module state across revisions; document the actual build/cache lifetime or clarify loader invalidation.
🤖 Prompt for 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.
In `@comptime.md` around lines 278 - 279, Update the “Real module identity”
section to describe comptime-imported module instances as scoped to the active
build/cache lifetime rather than the compiler process. Align the statement with
the later cache contract by documenting that invalidation clears the module
state, and avoid claiming process-wide persistence across revisions.
| - `<-if>` removes code with a guarantee — including from analysis metadata, walks, and serialized state, which no downstream minifier could do. | ||
| - `<-for>` _duplicates_ its body program per iteration. The unroll-vs-inline guidance in the [`<-for>` reference](#-for) (bake structure, not repetition) is the primary authoring rule; `<-log>` plus the sizes tooling (`npm run build:sizes`, fixture `sizes.json`) make regressions visible in review. | ||
| - Materialized values dedup structurally per template: the serializer's reference graph hoists shared objects and long repeated strings to single `static const`s, the same guarantee the resume payload gives on the wire. Build-wide, the [shared-module store](#cross-template-dedup-through-the-build-cache) extends this across templates: N pages baking the same data ship one content-addressed module, deduplicated by the bundler like any other import. | ||
| - **Runtime cost is zero by construction** (principle 4): no runtime helper, no new runtime branch, no size change to `@marko/runtime-tags` for programs that never use comptime — and none for programs that do, either, since expansion output uses only existing constructs. The `.sizes.json` diff for the implementing commits should be exactly zero on the runtime side. |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Remove the guarantee that comptime adds zero runtime cost.
The document also specifies unrolled bodies, serialized values, and virtual modules/functions, all of which can add runtime bytes or work. Limit this claim to “no new runtime machinery” and state that generated output retains the normal cost of those constructs.
🤖 Prompt for 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.
In `@comptime.md` at line 805, Update the “Runtime cost is zero by construction”
statement in the comptime principles documentation to remove the zero-size and
zero-cost guarantee. Limit the claim to comptime introducing no new runtime
machinery, and state that generated output retains the normal runtime bytes and
work of its unrolled bodies, serialized values, and virtual modules/functions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds compile time evaluation to the tags translator, per the included design doc (
comptime.md):comptimestatements andcomptime imports evaluate once per template in the compiler's realm; module instances cache in the buildcache.<-if>,<-for>(of/in/range),<-const>,<-log>, and<-debug>expand during transform with document-order scoping and hygienic unrolling.<-my-tag>macro-expands the tag's template into the caller withinputknown at compile time — static parts fold away, runtime content (handlers, state, nested tags) splices through live, and the same file still works as a normal runtime tag.<-define>declares in-file snippets (invoked as<-Name>, or passed around as comptime values);<-return>binds the caller's tag variable to a compile-time result. Expansion carries per-file comptime programs, content fragments at<${input.content}/>sites, attribute-tag inputs, module/import splicing with hygiene, expansion traces in errors, and a fixed depth guard.__proto__keys stay data, and null prototypes round-trip. Primitive member reads fold to escaped static output; non-primitive reads serialize just the accessed subtree. Comptime-born functions cross as capture-applied factories emitted into a content-addressed virtual module viaresolveVirtualDependency(inline when no hook is configured).marko/comptimeshipsinert/isInert: an airlock that deep-snapshots untrusted data into frozen, null-prototype trees safe to hand to comptime evaluation.$ct-prefixed names are reserved in comptime code, keeping evaluator internals unreachable.errorRecoverycompiles lower constructs to runtime analogues so bodies analyze and type-check without evaluating.causewith statement-anchored code frames and expansion traces,<-log>surfaces through the compiler diagnostics channel, and compile results carrymeta.comptime.<-for>unrolling at 100k iterations, macro depth at 100, and user-code execution at 30s per template (VM-enforced, so even a hung loop terminates the build loudly).Checklist:
🤖 Generated with Claude Code
https://claude.ai/code/session_018gim3DpEogDyt7jKdFGuAY