chore: Introduce CTHelpers in ts-transformers serving imported symbols. #1892

jsantell · 2025-10-09T20:10:31Z

As a part of the "pretransform" step, we replace the CTS enable directive with a wildcard import for "commontools" module that is used when injecting new functionality via transformations.

Summary by cubic

Introduced a pretransform that replaces the CTS directive with import * as __ctHelpers from "commontools", and added CTHelpers so transformers access helpers reliably without custom import management. This simplifies helper resolution and runs before TypeScript binding to avoid symbol issues.

Refactors
- Added CTHelpers to find and access helpers like derive, ifElse, toSchema, and JSONSchema.
- Replaced imports.ts and cts-directive with ct-helpers; transformers now use ctHelpers instead of ImportRequirements.
- Updated runner to pretransform programs (inject helper import and prefix files) via pretransformProgram.
- Schema transformers detect both toSchema() and __ctHelpers.toSchema(), and use JSONSchema via a qualified name (no import edits).
- Tests pretransform sources with transformCtDirective.
Migration
- If using ts-transformers outside the runner, run transformCtDirective on sources (or add the __ctHelpers import) before compiling.
- Do not use the reserved identifier __ctHelpers in user code.

cubic-dev-ai

1 issue found across 17 files

Prompt for AI agents (all 1 issues)


Understand the root cause of the following 1 issues and fix them.


<file name="packages/ts-transformers/src/core/ct-helpers.ts">

<violation number="1" location="packages/ts-transformers/src/core/ct-helpers.ts:102">
`getCTHelpersIdentifier` should compare the string literal’s text, not its quoted source, so helpers added with single quotes are still detected.</violation>
</file>

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

packages/ts-transformers/src/core/ct-helpers.ts

mathpirate · 2025-10-09T22:15:42Z

packages/ts-transformers/src/core/ct-helpers.ts

+): string {
+  // Throw when this symbol name is already in use
+  // for some reason.
+  if (source.indexOf(CT_HELPERS_IDENTIFIER) !== -1) {


i think this is reasonable... just noticing it'll reject even on something like // TODO: include __ctHelpers or // note: we omit __ctHelpers or whatever. i can't really think of an important use case where this is a problem so i'm good with this as-is, mentioning for your consideration

i guess this also means it's not valid for users to manually write import * as __ctHelpers from commontools right?

Right, we could use ts API to create a unique name, or use the existence of any * import for commontools rather than a new one, though that works less well with the sourceline preservation I think, could try that too

mathpirate · 2025-10-09T22:21:10Z

packages/ts-transformers/src/core/ct-helpers.ts

+    name: string,
+  ): ts.PropertyAccessExpression {
+    if (!this.sourceHasHelpers()) {
+      throw new Error("Source file does not contain helpers.");


should we consider making this error message more suggestive, eg "Source file does not contain helpers. Did you mean to include /// <cts-enable />?

This occurs if the transformers filtering have failed, so it's on our end, not an author's

mathpirate · 2025-10-09T22:21:20Z

packages/ts-transformers/src/core/ct-helpers.ts

+    name: string,
+  ): ts.QualifiedName {
+    if (!this.sourceHasHelpers()) {
+      throw new Error("Source file does not contain helpers.");


same q here re error message potentially being more explicit

mathpirate · 2025-10-09T22:22:00Z

packages/runner/src/harness/pretransform.ts

+  return `/${id}${filename}`;
+}
+
+function isCTSEnabled(line: string) {


unused (i think you moved it elsewhere)

mathpirate · 2025-10-09T22:23:38Z

packages/ts-transformers/src/transformers/builtins/derive.ts

@@ -1,31 +1,5 @@
 import ts from "typescript";
-import {


very nice simplifications here and throughout!

mathpirate · 2025-10-09T22:27:01Z

packages/ts-transformers/src/transformers/schema-generator.ts

+
+// Helper type extending CallExpression with
+// truthy typeArguments.
+interface ToSchemaNode extends ts.CallExpression {


cool thank you this is nicer

mathpirate · 2025-10-09T22:27:34Z

packages/ts-transformers/src/transformers/schema-generator.ts

-        const jsonSchemaIdentifier = context.imports.getIdentifier(
-          context,
-          { module: "commontools", name: "JSONSchema" },
+        const jsonSchemaName = context.ctHelpers.getHelperQualified(


i had to ask claude for some help understanding the reason for getHelperQualified here. ultimately it suggests that something like the following comment might be helpful, slash, is this in fact the correct understanding?

"// Use getHelperQualified() for type positions (satisfies, type references)
// vs getHelperExpr() for value positions (function calls, property access)"

Mostly due to that's what the SatisfiesExpression needs, as its a const statement, and when we use the expression form, it's for identifiers which could represent values or functions etc. -- TBD on what other Node forms we'll need to offers

mathpirate · 2025-10-09T22:29:25Z

packages/ts-transformers/src/transformers/opaque-ref-jsx.ts

  detectCallKind,
  isEventHandlerJsxAttribute,
 } from "../ast/mod.ts";
-import { OpaqueRefHelperName, rewriteExpression } from "./opaque-ref/mod.ts";


hm was this previously an unused import? i'd thought typescript would have complained about that

mathpirate · 2025-10-09T22:30:41Z

packages/ts-transformers/src/transformers/opaque-ref/emitters/conditional-expression.ts

@@ -1,6 +1,6 @@
 import ts from "typescript";

-import type { Emitter, OpaqueRefHelperName } from "../types.ts";


same q here, previously unused?

mathpirate · 2025-10-09T22:35:31Z

one general DX question... since the runner is where we do the pretransform, is there any nice way to warn users if they seem to be trying to use the transformers outside of the runner? eg if a user has /// in their source file but they try to run the transformers pipeline without sending it through the runner, is there any nice way we can indicate to them that that's probably their issue?

the only idea i had would be to check in each transformer's filter() function, which is pretty ugly and requires us, transformer authors, to remember to do that for each new transformer sub-class, so i don't love it.

otherwise i guess they'll just get typescript compiler errors about missing the injected functions?

mathpirate · 2025-10-09T22:36:38Z

one other general DX question... if a user manually imports derive from commontools, and then uses it - will there be two differently qualified versions of derive in the JS that the AMD loader generates? it's probably fine right?

mathpirate · 2025-10-09T22:40:42Z

this looks really great! thank you so much for figuring this out and continuing to make everything in this part of the codebase so much nicer.

jsantell · 2025-10-09T23:09:58Z

one other general DX question... if a user manually imports derive from commontools, and then uses it - will there be two differently qualified versions of derive in the JS that the AMD loader generates? it's probably fine right?

Yes, exactly!

jsantell · 2025-10-09T23:21:03Z

one general DX question... since the runner is where we do the pretransform, is there any nice way to warn users if they seem to be trying to use the transformers outside of the runner? eg if a user has /// in their source file but they try to run the transformers pipeline without sending it through the runner, is there any nice way we can indicate to them that that's probably their issue?

the only idea i had would be to check in each transformer's filter() function, which is pretty ugly and requires us, transformer authors, to remember to do that for each new transformer sub-class, so i don't love it.

otherwise i guess they'll just get typescript compiler errors about missing the injected functions?

Yeah it's tricky, ts-transformers is a part of the compilation step which also must have been preprocessed with runner-specific information. Two-pass compiling makes this nicer, at the cost of 50% slower compiles, but definite room for improvement here

As a part of the "pretransform" step, we replace the CTS enable directive with a wildcard import for "commontools" module that is used when injecting new functionality via transformations. Remove unused imports from various patterns that are now injected by the transformers.

mathpirate

yet another big improvement thank you

cubic-dev-ai bot reviewed Oct 9, 2025

View reviewed changes

packages/ts-transformers/src/core/ct-helpers.ts Outdated Show resolved Hide resolved

mathpirate reviewed Oct 9, 2025

View reviewed changes

jsantell force-pushed the ct-helpers-refactor branch from 54a70dc to 03aa134 Compare October 9, 2025 22:57

jsantell temporarily deployed to ci October 9, 2025 23:01 — with GitHub Actions Inactive

jsantell force-pushed the ct-helpers-refactor branch 2 times, most recently from e79728e to d93c319 Compare October 10, 2025 16:34

jsantell force-pushed the ct-helpers-refactor branch from d93c319 to 22a9a0d Compare October 10, 2025 16:37

jsantell temporarily deployed to ci October 10, 2025 16:41 — with GitHub Actions Inactive

mathpirate approved these changes Oct 10, 2025

View reviewed changes

jsantell merged commit c097738 into main Oct 10, 2025
8 checks passed

jsantell deleted the ct-helpers-refactor branch October 10, 2025 17:29

		@@ -1,6 +1,6 @@
		import ts from "typescript";

		import type { Emitter, OpaqueRefHelperName } from "../types.ts";

chore: Introduce CTHelpers in ts-transformers serving imported symbols. #1892

chore: Introduce CTHelpers in ts-transformers serving imported symbols. #1892

Uh oh!

Conversation

jsantell commented Oct 9, 2025 • edited by cubic-dev-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by cubic

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mathpirate commented Oct 9, 2025

Uh oh!

mathpirate commented Oct 9, 2025

Uh oh!

mathpirate commented Oct 9, 2025

Uh oh!

jsantell commented Oct 9, 2025

Uh oh!

jsantell commented Oct 9, 2025

Uh oh!

mathpirate left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jsantell commented Oct 9, 2025 •

edited by cubic-dev-ai bot

Loading