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 undefined steps for partial workflow #5

Merged
merged 3 commits into from
May 30, 2024
Merged

Fix undefined steps for partial workflow #5

merged 3 commits into from
May 30, 2024

Conversation

emcfarlane
Copy link
Collaborator

@emcfarlane emcfarlane commented May 30, 2024

Have an error report of: Error: Cannot read properties of undefined (reading 'status'). I believe it's from the steps[key].status == Status.Failed if the value is in the object but undefined. Removed the class for an object that is allowed to be partial and added more ?. guards for field access.

@emcfarlane emcfarlane requested a review from paul-sachs May 30, 2024 20:15
Copy link

github-actions bot commented May 30, 2024

The latest Buf updates on your PR.

NameStatus
build✅ passed
lint⏩ skipped
format❌ failed
breaking✅ passed

src/main.ts Outdated
@@ -55,8 +54,8 @@ async function main() {
// NB: Write empties the buffer must be after the comment.
await summary.write();
// Finally, set the status of the action.
for (const key of Object.keys(steps)) {
if (steps[key].status == Status.Failed) {
for (const [key, value] of Object.entries(steps)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Object.entries looses the type and goes to string, any.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, unfortunately typescript is representing javascript correctly here. Steps could contain other things. You could help it be explicit with something like:

 for (const [key, value] of Object.entries(steps) as [
    keyof Steps,
    Result | undefined
  ][]) {

Will suffice to make TS happy.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! Added the type sig, but still don't get an error on value.status vs value?.status. Seems like the value doesn't respect the undefined or it makes no diff to the type check here?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, that is weird, haven't seen it such that it coerced. Might be a TS option that I'm missing somewhere.

@emcfarlane emcfarlane requested a review from nicksnyder May 30, 2024 22:25
@emcfarlane emcfarlane merged commit fc0b591 into main May 30, 2024
26 checks passed
@emcfarlane emcfarlane deleted the ed/debugSteps branch May 30, 2024 23:28
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