fix(#650): allows multiple setvalue actions to reference the same element#651
Conversation
🦋 Changeset detectedLatest commit: 4fb35d9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
|
I'll review this and test it first thing tomorrow |
latin-panda
left a comment
latin-panda
left a comment
There was a problem hiding this comment.
Nice! Works well. I've added a small suggestion below
| if (actions) { | ||
| for (const action of actions) { | ||
| dispatchAction(context, setValue, action); | ||
| } | ||
| } |
There was a problem hiding this comment.
This is fine, but you can also consider this flatter version:
| if (actions) { | |
| for (const action of actions) { | |
| dispatchAction(context, setValue, action); | |
| } | |
| } | |
| actions?.forEach((action) => dispatchAction(context, setValue, action)); |
| const key = ModelActionMap.getKey(action.ref); | ||
| this.set(key, action); | ||
| if (this.has(key)) { | ||
| this.get(key)!.push(action); |
There was a problem hiding this comment.
I noticed that TypeScript has been less good at understanding the code and requiring non-null assertions (!) more often. I'll check the TypeScript config to see if there's anything that could improve that.
There was a problem hiding this comment.
Apparently this is not new: microsoft/TypeScript#9619
One option is to just call get instead of has and check if the result is null but this feels cleaner to me...
There was a problem hiding this comment.
I also prefer using has
2 participants
Closes #650
I have verified this PR works in these browsers (latest versions):
What else has been done to verify that this works as intended?
Why is this the best possible solution? Were any other approaches considered?
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
Do we need any specific form for testing your changes? If so, please attach one.
What's changed