-
Notifications
You must be signed in to change notification settings - Fork 9
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
Remove accidental mutation #520
Conversation
🦋 Changeset detectedLatest commit: 21b622f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a changeset, other than that LGTM :) Nice find.
const stepWithError = t.events[phase].reverse().find(t => t.data.error); | ||
if (stepWithError) { | ||
t.error = stepWithError.data.error; | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Array.prototype.reverse
mutates the events array in place. This is not what we want.
Furthermore it's kind of wasteful to call this method anyway. We could use Array.prototype.findLast
except I'm not sure what our minimum Node target is so...
Check out go/r/5bb1c16d-abc8-4c69-962a-9bef52b0efa9 for more.