fix(ts-sdk): emit v0 root-import deprecation via process.emitWarning#3933
fix(ts-sdk): emit v0 root-import deprecation via process.emitWarning#3933sushaan-k wants to merge 1 commit into
Conversation
|
@sushaan-k is attempting to deploy a commit to the Hatchet Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Updates the TypeScript SDK’s root-import v0 deprecation messaging to use Node’s standard warning mechanism (process.emitWarning) so consumers can suppress/dedupe/filter warnings using built-in Node flags and warning handlers.
Changes:
- Introduces a
emitV0RemovedWarning(submodule, detail?)helper that emitsDeprecationWarningwith codeHATCHET_V0_REMOVED(with aconsole.warnfallback). - Replaces the module-evaluation
console.warnblocks inworkflow.tsandstep.tswith a single helper call each. - Adds unit tests covering the warning shape, code, per-submodule dedupe, and fallback behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sdks/typescript/src/workflow.ts | Replaces v0 root-import console.warn banner with a single structured deprecation warning emission. |
| sdks/typescript/src/step.ts | Replaces v0 root-import console.warn banner with a single structured deprecation warning emission. |
| sdks/typescript/src/util/v0-deprecation-warning.ts | Adds centralized warning emission + per-submodule dedupe and fallback logic. |
| sdks/typescript/src/util/v0-deprecation-warning.test.ts | Adds Jest coverage for emission, code, dedupe, and fallback paths. |
| /** Reset hook for tests — not part of the public API. */ | ||
| export function _resetEmittedV0Warnings(): void { |
There was a problem hiding this comment.
@sushaan-k I agree this is technically deep-importable because the package has no exports map, but I do not see it as a practical blocker since it is not re-exported from the root package and the function is only a harmless test reset hook.
Non-blocking suggestion: adding @internal to the JSDoc would make that intent clearer in the generated declarations.
| * anyone importing from the root, since `index.ts` re-exports both). | ||
| * | ||
| * Switching to `process.emitWarning` with a fixed code makes the warnings | ||
| * filterable, dedupable, and consistent with the rest of Node's deprecation |
| * to keep nagging consumers to migrate to v1, but the original implementation | ||
| * used `console.warn` at module evaluation time, which: | ||
| * - cannot be silenced by Node's standard `--no-deprecation`, | ||
| * `--no-warnings`, or `--no-warnings=DeprecationWarning` flags; |
There was a problem hiding this comment.
non-blocking suggestion: I don’t think this source comment needs to enumerate the exact Node CLI flags. Since those are Node-owned behavior and can vary by version, I would slightly prefer keeping the source comment focused on the reason for process.emitWarning: it gives consumers structured warning metadata and a stable HATCHET_V0_REMOVED code that standard Node warning controls can filter or suppress. That would also avoid teaching the undocumented --no-warnings=DeprecationWarning form in source.
|
@sushaan-k thank you for the PR, this looks good to me. I validated the warning behavior locally and this addresses #3915:
I also confirmed the warnings can be suppressed through standard Node warning mechanisms such as I left two non-blocking suggestions to improve JSDoc and to mark the test reset helper as internal, but neither blocks approval from my perspective. |
|
@sushaan-k one release housekeeping request before merge: could you bump the TypeScript SDK package version in PR #3932 can add its own separate changelog entry under the same version. |
7d2e373 to
c43b395
Compare
…ning
The root export still pulls in legacy workflow and step submodules, which
spammed seven raw console.warn lines on every process start. Those
messages ignored --no-deprecation / --no-warnings and had no stable code,
so process.on('warning') handlers had nothing to match on.
Route them through process.emitWarning with code HATCHET_V0_REMOVED and
dedupe per submodule via a small helper. The warning still shows up by
default but now respects Node's standard deprecation surface.
emitWarning queues 'throw warning' on the next tick under
--throw-deprecation or process.throwDeprecation, so a try/catch around
the call would not catch it. That would abort the root import for
consumers who never opted into the v0 surface but pull it in
transitively via index.ts. Check the flag up front and route to
console.warn in that case, and also when the host runtime does not
expose process.emitWarning at all.
Closes hatchet-dev#3915
c43b395 to
d9bdd22
Compare
Hi! Picking up #3915.
Importing
@hatchet-dev/typescript-sdkfrom the root specifier currently prints seven redconsole.warnlines on every process start, becauseindex.tsre-exports both./workflowand./stepand each of those fires rawconsole.warncalls at module evaluation. They:--no-deprecation,--no-warnings,--no-warnings=DeprecationWarning),code, soprocess.on('warning', ...)handlers can't match on them,Change
Added a small helper
emitV0RemovedWarning(submodule, detail?)insdks/typescript/src/util/v0-deprecation-warning.ts. It:Set.process.emitWarning(..., { type: 'DeprecationWarning', code: 'HATCHET_V0_REMOVED' })when available, so the standard Node flags work andprocess.on('warning', e => e.code === 'HATCHET_V0_REMOVED')is a stable filter.process.throwDeprecation(set by--throw-deprecationor directly) up front and routes toconsole.warninstead when true. Without this guard the root import would abort for transitive v0 consumers, becauseemitWarningqueuesthrow warningon the next tick after it returns — atry/catcharound the call cannot catch it. Same fallback runs on runtimes that don't exposeprocess.emitWarningat all.A
try/catcharound the emit remains as defense-in-depth for non-Node hosts where a polyfilledemitWarningcould throw synchronously.workflow.tsandstep.tseach replace their four-lineconsole.warnblock with a single call to the helper. The migration nag is still visible by default.Testing
Tests cover:
HATCHET_V0_REMOVEDcode,detailpass-through,process.throwDeprecation = true→ bypassesemitWarning, goes toconsole.warn, no crash,console.warnfallback whenprocess.emitWarningis unavailable.Also smoke-tested against real Node 24 with each of the relevant flags. Output for each invocation, exit code 0 in every case:
A note on shape
Matched the
HATCHET_V0_REMOVEDcode the issue suggested to keep review easy. Happy to split into per-submodule codes (HATCHET_V0_WORKFLOW_REMOVED,HATCHET_V0_STEP_REMOVED) if you'd prefer finer-grained filtering.I considered the existing
emitDeprecationNoticehelper inv1/client/worker/deprecated/deprecation.ts, but it takes aLoggerinstance and the v0 warnings fire at module-evaluation time with no logger context. The issue also explicitly called forprocess.emitWarning, so a small dedicated helper felt cleaner than threading a logger into module scope.Closes #3915