chore: drop chalk in favor of node:util styleText - #1934
Draft
toiroakr wants to merge 946 commits into
Draft
Conversation
…ction Also describe the refined injection behavior in the cli-plugins changeset.
Version Packages (next)
The .validate() callback's issues() field parameter used a self-generic
<P extends DottedPaths<...>> signature. Comparing two such generic call
signatures requires TypeScript to check the inner constraint for identity
rather than assignability, which spuriously rejected structurally-equal
TailorDBType instances built from the same custom fields but derived
through different generic instantiation paths (e.g. a table passed across
a module factory boundary), surfacing as TS2719 ("Two different types
with this name exist, but they are unrelated"). Making the parameter a
plain (non-generic) union keeps strict field-path checking while using
ordinary assignability for these comparisons.
Comment restated design rationale already covered by the commit message and regression test; per the code-comments convention, source comments should express constraints the code can't otherwise convey, not PR rationale.
Rename to make clear this documents assignability when a factory's parameter type is derived via explicit type-argument passthrough, rather than serving as a direct regression guard for the TS2719 false-positive reported upstream. A standalone repro of that exact compiler behavior could not be constructed without the consuming package's full module composition.
Resolve conflicts by taking v2's dependency bumps and generated workflow types, and drop the pkg.pr.new zinfer override now that zinfer 0.2.5 includes commit 054a3a5.
fix(tailordb): make issues() field param non-generic
… to .start() Remove the pre-alignment `tailor.workflow` names `triggerWorkflow`, `triggerJobFunction`, and `resumeWorkflow` (and their `TriggerWorkflowOptions` / `TriggerJobFunctionOptions` option types) from `@tailor-platform/sdk/runtime`, the ambient `@tailor-platform/sdk/runtime/globals` types, and the `mockWorkflow()` test facade. Use the canonical `startWorkflow`, `startJobFunction`, and `resumeWorkflowExecution` names instead. Also rename `Workflow.trigger()` (from `createWorkflow()`) and `WorkflowJob.trigger()` (from `createWorkflowJob()`) to `.start()`, aligning the SDK's ergonomic verb with the platform's `start*` RPC vocabulary. `mockWorkflow()`'s `wf.job()`/`wf.workflow()` mocks, `wf.setTriggerHandler` / `wf.triggeredJobs`, and the CLI bundler's trigger detection/codegen are updated to match. Adds the `v2/workflow-trigger-rename` codemod (automatic) for the low-level rename, and registers `v2/workflow-start-rename` (manual, guided by prompt) for the `.trigger()` -> `.start()` call-site rename, since distinguishing a workflow/job `.trigger()` call from an unrelated object's own method isn't reliably decidable from a single file's syntax.
…rgs-contract fix(executor): align workflow argument contracts
`docs/services/workflow.md` links to `example/resolvers/startWorkflow.ts` on `main`, but that file only exists on the `v2` branch until it merges. Exclude the URL from lychee's link check so this known, temporary, non-blocking gap doesn't fail CI; it self-resolves once `v2` lands on `main`.
# Conflicts: # packages/sdk/src/configure/services/executor/executor.ts
- Rename the internal `getTriggerCallInfo`/`TriggerCallInfo` AST helper to `getStartCallInfo`/`StartCallInfo`: it now detects `.start(...)` calls, and its JSDoc already said so, so the old name was misleading next to it. - Fix `packages/sdk/src/vitest/globals.ts`'s comment on the default workflow runtime: it doesn't run the real job chain locally (that's `runWorkflowLocally()`) — job/wait/resolve calls throw a helpful error by default, and workflow starts return a placeholder execution id. - Fix "in separated job" -> "in a separate job" in docs/services/workflow.md.
…ch helper Share the plugin slug computation between dispatch and the install hint, and move the hint branch out of runMain so it can be unit tested.
The v2 branch releases under the npm next dist-tag while changesets pre-release mode is active; a plain install would resolve to the 0.0.0 bootstrap version on latest.
- The bundler's and trigger-transformer's `.start(` pre-checks used a plain substring match, which misses whitespace/newline-formatted calls (e.g. `.start (` or `.start\n(`) and would silently skip the AST transform, leaving an untransformed `.start(...)` call to throw at runtime in the bundle. Add a shared `hasStartCall()` regex helper and use it in all three call sites (trigger-transformer.ts, bundler.ts x2). - Fix stale "trigger"/"triggerJobFunction" wording in ast-transformer.test.ts comments and titles that no longer match the `startJobFunction` assertions they describe.
The v2 merge bumped tsdown to 0.22.5 everywhere else, dropping the 0.22.4 lockfile entry the plugin still referenced and breaking frozen-lockfile installs.
install-deps now consumes the pending-rebuild state while the plugin's dist is still missing, so the later rebuild --pending no longer relinks the skipped bin. Linking at install time against a stub removes the relink dependency entirely.
…h the public API rename Rename trigger-context.ts/trigger-transformer.ts and their exported identifiers (TriggerContext, TriggerTarget, TriggerModuleBindings, ResolvedTriggerCall, transformFunctionTriggers, createTriggerTransformPlugin, detectResolvedTriggerCalls, etc.) to their Start* equivalents, aligning internal AST/bundler naming with the .start() rename. Unrelated Trigger usages (executor/IdP event triggers) are untouched.
…kflow-trigger-dispatch sdk-codemod's CLI applies all applicable codemods in a single --from/--to pass. v2/workflow-start-rename has no scriptPath, so it never auto-transforms .trigger() to .start() within that pass, and pre-migration v1 code cannot contain .start( at all (the method did not exist under that name). The scenario this pattern guarded against — a file already renamed to .start( before workflow-trigger-dispatch's suspiciousPatterns are checked — is unreachable in the tool's actual single-shot usage.
…lugin feat(sdk)!: extract tailordb erd into a CLI plugin package
This comment has been minimized.
This comment has been minimized.
renderFor took a "stdout" | "stderr" target, and the spinner mapped its configurable stream onto one of them, so every stream other than process.stdout followed stderr's color support. A spinner pointed at a file stream would have written escape codes into the file whenever stderr happened to be a terminal. renderFor now takes the stream and asks styleText about that stream directly. An instanceof Stream guard keeps the probe from rejecting test doubles, which are treated as colorless as before. This drops the OutputTarget type and its mapping helper, so there is one fewer place for a destination and its color decision to disagree.
This comment has been minimized.
This comment has been minimized.
The color factory and the per-stream strip lived in four copies, one per package that writes styled output. Since they encode one rule — always emit escapes, drop them for destinations without color support — four copies is four chances for that rule to drift. They now live in @tailor-platform/shared, a private package consumed as a devDependency and bundled into each consumer, following the same wiring as @tailor-platform/tailor-proto. Nothing about it reaches consumers of the published packages: the built output inlines it, and only sourcemaps mention the package name. Subpaths keep the package from becoming a junk drawer as more moves in; ./color is the only one for now.
This comment has been minimized.
This comment has been minimized.
Styling moved from chalk, which ran anywhere, to node:util styleText, which needs the stream and validateStream options added in Node 20.12. Only the sdk package declared an engine range, and the codemod was still built for node18, so installing it on Node 18 or 20 would have produced a missing-function error at the first colored line instead of a version mismatch at install time. sdk-codemod, sdk-plugin-seed and sdk-plugin-tailordb-erd now declare the same range as the sdk package, and the codemod builds for node22 like the rest. Also corrects the comment above the color-support probe: styleText throws for values that are not streams, which is why the instanceof check filters test doubles out before the call rather than letting styleText handle them.
This comment has been minimized.
This comment has been minimized.
The shared module exported a curried factory, so every consumer had to name the
result before using it: the codemod kept four module-level aliases whose only
job was to turn color("bold") into bold. Exporting the style functions keyed by
name removes that step — call sites now read color.bold(text), and the codemod
passes color.green to its line writer instead of aliasing it first.
The names are listed explicitly rather than derived from util.inspect.colors.
That list is missing five names styleText nevertheless accepts (grey,
blackBright, bgGrey, bgBlackBright, none), so deriving from it while typing the
result from the styleText parameter union would hand out properties that are
undefined at runtime.
The one compound style, the gray italic placeholder, composes instead:
color.italic(color.gray(text)). styleText reopens the outer style after the
inner one closes, so the rendering is unchanged.
This comment has been minimized.
This comment has been minimized.
Importing the shared color module made rolldown-plugin-dts warn on every sdk build: it resolves declarations lazily and cannot do so for a TypeScript source file outside this package. The declaration output is identical either way — 258 files before and after, and every types target in package.json#exports is present in both — so the warning was noise, but noise on every build teaches people to skip reading it. Enabling the eager option the warning itself recommends costs about three seconds of build time. Shipping compiled output from the shared package would also silence it, at the price of a dist that can go stale against its source.
Code Metrics Report (packages/sdk)
Details | | main (c5e1a27) | #1934 (1351c5a) | +/- |
|--------------------|----------------|-----------------|-------|
+ | Coverage | 75.4% | 77.7% | +2.2% |
| Files | 467 | 459 | -8 |
| Lines | 17741 | 17485 | -256 |
+ | Covered | 13390 | 13589 | +199 |
+ | Code to Test Ratio | 1:0.4 | 1:0.4 | +0.0 |
| Code | 120229 | 123407 | +3178 |
+ | Test | 56943 | 59749 | +2806 |Code coverage of files in pull request scope (77.3% → 83.6%)Skip file coverages because there are too many files (103) SDK Configure Bundle Size
Runtime Performance
Type Performance (instantiations)
Reported by octocov |
toiroakr
force-pushed
the
v2
branch
3 times, most recently
from
July 31, 2026 09:51
77bc78c to
a3c4978
Compare
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
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.
Summary
Remove
chalkfrom the runtime dependencies of@tailor-platform/sdk,@tailor-platform/sdk-codemod,@tailor-platform/sdk-plugin-seedand@tailor-platform/sdk-plugin-tailordb-erd. It was used purely for terminal styling, whichnode:utilstyleTextcovers.chalk@5is now absent from the lockfile; the remainingchalk@4comes frommadge/orain devDependencies only, so the published dependency closure is chalk-free.Decide colour support per output stream.
chalkresolved one level from stdout and applied it to every string regardless of destination, andstyleTextvalidates against stdout by default, so redirecting stdout dropped the colours from diagnostics that were still going to a terminal, while redirecting stderr wrote escape codes into the log file. Style functions now always emit escapes and each write boundary renders for the stream it writes to — stderr for log lines, spinner frames and codemod diffs, stdout forlogger.outdata and the seed confirmation prompt. Detection is delegated tostyleTextitself, so the TTY /NO_COLOR/FORCE_COLORrules stay Node's.One implementation of that rule. It started as four copies, one per package that writes styled output, and now lives in
@tailor-platform/shared— a private package consumed as a devDependency and bundled into each consumer, wired the same way as@tailor-platform/tailor-proto. Nothing about it reaches consumers of the published packages: the built output inlines it, and only sourcemaps mention the package name. Subpaths (./colortoday) keep it from becoming a junk drawer.Declare the runtime each package needs.
styleText'sstream/validateStreamoptions arrived in Node 20.12, but only the sdk package declared an engine range, and the codemod was still built fornode18.@tailor-platform/sdk-codemod,@tailor-platform/sdk-plugin-seedand@tailor-platform/sdk-plugin-tailordb-erdnow declarenode >=22.15.0/bun >=1.2.0like the sdk package, so an old runtime reports a version mismatch at install time rather than a missing function at the first coloured line.Drop the now-dead
chalkentry from the CLIno-restricted-importsrule; thestyleTextrestriction stays, withlogger.ts/errors.tsremaining the only exempt files.Notes
Incidental findings from the migration:
cli/commands/function/logs.tspassedstring | undefinedinto a style function.chalk's signature acceptedunknown[], so the destructuredheaderLinenever had to be narrowed; the stricter(text: string) => stringtype surfaces it.sdk-plugin-seed'sapply.test.tsreplaced the whole logger module with a factory that only providedlogger, which broke onceapply.tsalso importedstyles. The mock now spreadsimportOriginal()and overrides onlylogger, so a new export cannot break it again.formatLogLine) or compare against thesymbolsconstants rather than literal glyphs (deploy plan formatters). Those assertions previously depended on vitest running outside a TTY, so they would have failed underFORCE_COLOR=1.ascii-tableand the spinner already measure width with ANSI sequences stripped.