Skip to content

Conversation

@mjameswh
Copy link
Contributor

@mjameswh mjameswh commented Nov 3, 2023

What changed

  • In a few places, we reflectively visit and transform our protobuf definitions. To determine the type of node being visited (Root, Namespace or Type), we would previously check the object's constructor.name. However, that name could get mangled if the package was bundled and minimized, which would generally result in error messages such as:

    <app-dir>/node_modules/@temporalio/client/src/types.ts:62
    export const { WorkflowService } = proto.temporal.api.workflowservice.v1;
                                                      ^
    TypeError: Cannot read properties of undefined (reading 'api')
    

    We now check constructor.className instead, which is a convenient property added by the protobufjs library, specifically for the purpose of reflection.

Note

  • We provide no guarantee regarding the inclusion of @temporalio/* packages as part of bundled and/or minimized applications. We do not test such usage at this time. We strongly recommend configuring your bundler to treat these packages as external.

@mjameswh mjameswh requested a review from a team as a code owner November 3, 2023 14:25
Copy link
Member

@bergundy bergundy left a comment

Choose a reason for hiding this comment

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

Let's always use className but otherwise LGTM.
Thanks!

@mjameswh mjameswh merged commit 4a69632 into temporalio:main Nov 15, 2023
@iamnafets
Copy link

iamnafets commented Nov 17, 2023

Wow this bug driving me absolutely crazy, thanks for the PR.

Adding a next.js PR to make this package external by default:
vercel/next.js#58573

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