fix(core): drop unique-symbol brand on LocalsKey to fix dual-package builds#3626
Conversation
…builds LocalsKey<T> was branded with a module-level `declare const __local: unique symbol`. tshy emits separate .d.ts files for the ESM and CJS outputs, and each gets its own `declare const __local: unique symbol` — TypeScript treats every such declaration as a nominally distinct type. Under certain pnpm hoisting layouts a single TypeScript compilation can resolve LocalsKey from both the ESM source path and the CJS dist path within the same call site. With unique-symbol brands the two variants are structurally incompatible — TS rejects passing one to a function that expects the other, with a misleading 'Property [__local] is missing' error. Replace the symbol brand with an optional phantom value-type field. T is still carried at the type level, the runtime shape is unchanged, and the ESM and CJS .d.ts outputs are now identical.
🦋 Changeset detectedLatest commit: df1cad3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 32 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 |
WalkthroughThis PR fixes a TypeScript type compatibility issue with Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/core/src/v3/locals/types.ts (1)
1-20: ⚡ Quick winAdd
@crumbsmarkers to this changed block.The edits around
LocalsKey<T>don’t include//@Crumbs(or a `// `#region` `@crumbswrapper). Please annotate this block per workflow.As per coding guidelines, “Add crumbs as you write code — not just when debugging. Mark lines with
//@Crumbsor wrap blocks in `// `#region` `@crumbs.”🤖 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/core/src/v3/locals/types.ts` around lines 1 - 20, The LocalsKey<T> declaration block is missing the required crumbs annotations; add a crumbs marker to this changed block by inserting a comment marker (either a line comment // `@crumbs` on the declaration or wrap the block with // `#region` `@crumbs` ... // `#endregion` `@crumbs`) immediately surrounding the export type LocalsKey<T> (including its id, __type and __valueType members) so the block is annotated per the project's crumb workflow.packages/core/src/v3/locals/manager.ts (1)
6-8: ⚡ Quick winPlease add
@crumbsannotations for these edits.Both updated return-object blocks are missing breadcrumb markers (
//@Crumbsor `// `#region` `@crumbs).As per coding guidelines, “Add crumbs as you write code — not just when debugging. Mark lines with
//@Crumbsor wrap blocks in `// `#region` `@crumbs.”Also applies to: 24-26
🤖 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/core/src/v3/locals/manager.ts` around lines 6 - 8, The updated return-object blocks that set "__type: Symbol()" and "id" are missing breadcrumb annotations; add inline breadcrumb comments (e.g., "// `@crumbs`") or wrap the return-object blocks with "// `#region` `@crumbs`" markers around the object literal(s) that include "__type" and "id" so they follow the project's guideline; make the same change for the other block referenced around lines 24-26 to ensure both return-object sections in manager.ts are annotated.
🤖 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.
Nitpick comments:
In `@packages/core/src/v3/locals/manager.ts`:
- Around line 6-8: The updated return-object blocks that set "__type: Symbol()"
and "id" are missing breadcrumb annotations; add inline breadcrumb comments
(e.g., "// `@crumbs`") or wrap the return-object blocks with "// `#region` `@crumbs`"
markers around the object literal(s) that include "__type" and "id" so they
follow the project's guideline; make the same change for the other block
referenced around lines 24-26 to ensure both return-object sections in
manager.ts are annotated.
In `@packages/core/src/v3/locals/types.ts`:
- Around line 1-20: The LocalsKey<T> declaration block is missing the required
crumbs annotations; add a crumbs marker to this changed block by inserting a
comment marker (either a line comment // `@crumbs` on the declaration or wrap the
block with // `#region` `@crumbs` ... // `#endregion` `@crumbs`) immediately surrounding
the export type LocalsKey<T> (including its id, __type and __valueType members)
so the block is annotated per the project's crumb workflow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7a86ee36-24df-4499-a08c-2e13ba674355
📒 Files selected for processing (3)
.changeset/locals-key-dual-package-fix.mdpackages/core/src/v3/locals/manager.tspackages/core/src/v3/locals/types.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: typecheck / typecheck
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Import from@trigger.dev/coresubpaths only, never from the root. Subpath imports must be used to maintain proper module boundaries.
When writing Trigger.dev tasks, always import from@trigger.dev/sdk. Never use@trigger.dev/sdk/v3or deprecatedclient.defineJob.
Prisma is version 6.14.0. Use the Prisma client frominternal-packages/databasefor all database operations.
For ClickHouse client, schema migrations, and analytics queries, useinternal-packages/clickhouse.
Files:
packages/core/src/v3/locals/types.tspackages/core/src/v3/locals/manager.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
packages/core/src/v3/locals/types.tspackages/core/src/v3/locals/manager.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Add crumbs as you write code — not just when debugging. Mark lines with
//@Crumbsor wrap blocks in `// `#region` `@crumbs. They stay on the branch throughout development and are stripped byagentcrumbs stripbefore merge.
Files:
packages/core/src/v3/locals/types.tspackages/core/src/v3/locals/manager.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
packages/core/src/v3/locals/types.tspackages/core/src/v3/locals/manager.ts
packages/core/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/core/CLAUDE.md)
Never import the root package (
@trigger.dev/core). Always use subpath imports such as@trigger.dev/core/v3,@trigger.dev/core/v3/utils,@trigger.dev/core/logger, or@trigger.dev/core/schemas
Files:
packages/core/src/v3/locals/types.tspackages/core/src/v3/locals/manager.ts
**/*.{ts,tsx,js,jsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code formatting is enforced using Prettier. Run
pnpm run formatbefore committing
Files:
packages/core/src/v3/locals/types.tspackages/core/src/v3/locals/manager.ts
packages/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
When modifying any public package (
packages/*orintegrations/*), add a changeset usingpnpm run changeset:add. Default to patch for bug fixes and minor changes; confirm with maintainers before selecting minor; never select major without explicit approval.
Files:
packages/core/src/v3/locals/types.tspackages/core/src/v3/locals/manager.ts
🧠 Learnings (2)
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
packages/core/src/v3/locals/types.tspackages/core/src/v3/locals/manager.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
packages/core/src/v3/locals/types.tspackages/core/src/v3/locals/manager.ts
🔇 Additional comments (1)
.changeset/locals-key-dual-package-fix.md (1)
1-6: LGTM!
2 participants
Summary
LocalsKey<T>(the type returned bylocals.create()) was branded with amodule-level
declare const __local: unique symbol. Each such declarationis its own nominal type, and
tshyemits separate.d.tsfiles for theESM and CJS outputs — each gets its own
__localsymbol. Under certainpnpm hoisting layouts a single TypeScript compilation can resolve
LocalsKeyfrom both the ESM source path and the CJS dist path withinthe same call site, producing two structurally-incompatible variants of
the same type. TS surfaces this as the misleading error:
The error has been hitting CI on PRs opened since the chat.agent stack
landed (e.g. #3625 typecheck job), but doesn't reproduce on developer
machines where the pnpm node_modules layout was built up incrementally.
Fix
Replace the
unique symbolbrand with an optional phantom field thatcarries
Tat the type level:The ESM and CJS
.d.tsoutputs now produce structurally identical types,so cross-output resolution no longer produces a mismatch.
Tis stillcarried at the type level via the optional phantom field. The runtime
shape is unchanged —
manager.tswas already casting viaas unknown,which is no longer needed.
Test plan
pnpm run typecheck --filter @trigger.dev/core --filter @trigger.dev/sdkpnpm run build --filter @trigger.dev/core --filter @trigger.dev/sdk(clean rebuild) — confirms the ESM and CJS dist
.d.tsoutputsno longer carry distinct
unique symboldeclarationspnpm --filter @trigger.dev/core test test/mockTaskContext.test.ts --runpnpm --filter @trigger.dev/sdk test test/mockChatAgent.test.ts --run