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

Added triggered_id to clientside callbacks #2695

Merged
merged 8 commits into from
Dec 1, 2023

Conversation

AnnMarieW
Copy link
Collaborator

Closes #2692

@AnnMarieW AnnMarieW marked this pull request as draft November 18, 2023 17:03
@Coding-with-Adam
Copy link

Thanks for this PR, @AnnMarieW

@AnnMarieW
Copy link
Collaborator Author

You are welcome @Coding-with-Adam
I'm keeping it as a draft for now - need to figure out why so many tests fail. I'm hoping it's not related to the changes I made 🙂

@@ -576,6 +579,18 @@ function inputsToDict(inputs_list: any) {
return inputs;
}

function getTriggeredId(triggered: string[]): string | object {
Copy link
Contributor

Choose a reason for hiding this comment

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

The function needs to return something or add string | object | undefined to the return type.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @T4rk1n - can you point out where in the CI run you saw this build error?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a renderer build error in install-dependencies-36:

ERROR in /home/circleci/dash/dash/dash-renderer/src/actions/callbacks.ts
./src/actions/callbacks.ts 582:46-61
[tsl] ERROR in /home/circleci/dash/dash/dash-renderer/src/actions/callbacks.ts(582,47)
      TS2366: Function lacks ending return statement and return type does not include 'undefined'.
ts-loader-default_e3b0c44298fc1c14
 @ ./src/actions/index.js 6:0-52 72:11-32 111:21-42
 @ ./src/AppContainer.react.js 24:0-54 40:21-29 58:15-24
 @ ./src/AppProvider.react.tsx 11:0-48 21:38-50
 @ ./src/DashRenderer.js 8:0-46 15:50-61 19:54-65
 @ ./src/index.js 1:0-46 4:22-34

webpack 5.88.2 compiled with 1 error in 22846 ms
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! dash-renderer@1.17.0 build:js: `webpack`
npm ERR! Exit status 1
npm ERR! 

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, thanks. That's pretty buried - the step the actual error occurred in appears to succeed, then the step afterward fails cryptically. Worse, install-dependencies-39 has the same error but doesn't appear to fail at all (the logs moved?), so we try to run all the test jobs for Py3.9 and nearly everything fails, making it even harder to find the root cause.

Anything we can do to fix this, ie make the "Build Component Packages & Update Dependencies/Artifacts" step itself fail rather than relying on finding error logs to tell us it failed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, that worked. Thanks @T4rk1n

@AnnMarieW AnnMarieW marked this pull request as ready for review November 22, 2023 14:12
Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃 Looks perfect, thanks @AnnMarieW! If you can update the changelog to fix the conflict (caused by my patch release the other day), we'll merge 🎉

@alexcjohnson alexcjohnson merged commit 2153a68 into plotly:dev Dec 1, 2023
3 checks passed
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.

[Feature Request] Add triggered_id to clientside callabck_context
4 participants