Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Better error messages, and more #1126

Merged
merged 7 commits into from
May 30, 2023
Merged

Conversation

mjameswh
Copy link
Contributor

@mjameswh mjameswh commented May 26, 2023

What changed

@mjameswh mjameswh changed the title fix: Multiple minor fixes fix: Better error messages, and more May 29, 2023
@mjameswh mjameswh requested a review from bergundy May 29, 2023 21:33
const packageName = `@tsconfig/node${currentNodeVersion}`;
// For some reason, the @tsconfig/node20 package exists, but is currently unusable because it
// refers to a --lib value that isn't supported in latest release of TypeScript.
// FIXME: Update this once @tsconfig/node20 is fixed.
Copy link
Member

Choose a reason for hiding this comment

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

I'd go as far as linking the issue where there's a bit more information for future reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not actually a bug from their perspective. @tsconfig/node20 require TypeScript 5.x.

Updated comment accordingly.

Copy link
Member

Choose a reason for hiding this comment

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

Let’s upgrade to ts 5 in samples

Comment on lines 254 to 255
export function scheduleActivity<R>(activityType: string, args: any[], options: ActivityOptions): Promise<R> {
assertInWorkflowContext('Workflow.scheduleActivity(...) may only be used from a Workflow Execution');
Copy link
Member

Choose a reason for hiding this comment

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

These functions should have never been exported...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which function? scheduleActivity? I personally think it's a good thing that they are. There are situations where proxyActivities() is not a good fit, eg. when activity name is dynamic, or when users want to add their own wrappers around some activity calls... It's still possible to use proxies in those cases (eg. proxy[activityName](...)), but calling scheduleActivity directly makes things much more understandable.

@mjameswh mjameswh merged commit 4633b4f into temporalio:main May 30, 2023
23 checks passed
@mjameswh mjameswh deleted the misc branch May 30, 2023 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants