Skip to content

Fix TypeScript protobuf resolution - #474

Merged
stephanos merged 11 commits into
migrate-docker-images-to-trixiefrom
fix-typescript-protobufjs-resolution
Sep 8, 2026
Merged

Fix TypeScript protobuf resolution#474
stephanos merged 11 commits into
migrate-docker-images-to-trixiefrom
fix-typescript-protobufjs-resolution

Conversation

@stephanos

@stephanos stephanos commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Bump github.com/temporalio/features and TS SDK.

Alternatives considered:

  • SDK 1.23 without the direct, pinned dependency: fails with "type must be a Type".
  • SDK 1.22: fails because features overrides protobufjs to 8.x, while SDK 1.22 was built around the older protobuf stack.

@stephanos stephanos closed this Sep 7, 2026
@stephanos
stephanos deleted the fix-typescript-protobufjs-resolution branch September 7, 2026 20:20
@stephanos
stephanos restored the fix-typescript-protobufjs-resolution branch September 8, 2026 15:42
@stephanos stephanos reopened this Sep 8, 2026
@stephanos
stephanos changed the base branch from main to migrate-docker-images-to-trixie September 8, 2026 16:06
Comment thread go.mod
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.11.1
github.com/temporalio/features v0.0.0-20260427223549-86e4c0deedd7
github.com/temporalio/features v0.0.0-20260908141410-6f64678c5b39

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ie latest main

@stephanos
stephanos marked this pull request as ready for review September 8, 2026 16:24
@stephanos
stephanos requested review from a team as code owners September 8, 2026 16:24
Comment thread internal/workerctl/build.go Outdated
},
MoreDependencies: map[string]string{
"@grpc/proto-loader": "^0.8.0",
"protobufjs": "^8.7.1",

@stephanos stephanos Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The direct dependency is necessary because generated json-module.js performs require("protobufjs/light"). With pnpm, a transitive SDK dependency is not reliably visible there. Without a direct declaration, Node either cannot resolve it or finds the worker fixture’s protobufjs 7.x copy, producing incompatible runtime type identities.

@mjameswh
mjameswh self-requested a review September 8, 2026 18:04
@mjameswh

mjameswh commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

@stephanos You are missing dependencies in "workers/typescript/package.json". Should be something like this:

  "dependencies": {
    ...
    "protobufjs": "^8.7.1",
    "protobufjs-cli": "^2.6.1",
    ...
  },

"@temporalio/workflow": "^1.23.0",
"commander": "^11.1.0",
"long": "^5.2.3",
"protobufjs": "^8.8.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

webpackConfigHook(config) {
config.resolve!.alias = {
...config.resolve!.alias,
protobufjs: dirname(require.resolve('protobufjs/package.json')),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is unexpected (i.e. you should not need that), and quite surprising (i.e. even assuming you need it, it should not point to a package.json file).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's odd eh?

I'll leave this claim from my agent here for posterity:

Without the app.ts alias, source-SDK builds can bundle two physical protobufjs 8.8.0 copies:

  • Generated kitchen-sink protobuf types use the copy under prepared/node_modules.
  • The SDK payload converter uses the copy resolved through @temporalio/common.

Even though both are version 8.8.0, their Type constructors have different identities. The SDK’s instanceof check then throws:

TypeError: type must be a Type

Workflow activations repeatedly fail, so the five-noop smoke test appears stuck indefinitely. Released SDK layouts may pass due to package-manager hoisting, but the source-build CI layout
does not reliably deduplicate them. The build.go entry installs the dependency; the app.ts hook enforces one physical copy in the bundle.

@stephanos
stephanos merged commit 3bf8164 into migrate-docker-images-to-trixie Sep 8, 2026
44 checks passed
@stephanos
stephanos deleted the fix-typescript-protobufjs-resolution branch September 8, 2026 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants