Skip to content

Conversation

@rubendm92
Copy link
Contributor

Summary

I've added a function to create an empty effect ({}) in order to hide that the effects should be an object.
I think it is nice to hide that in case some day we wanted to change the effects to be an array, like in this PR #58

@mariosanchez
Copy link
Contributor

Right now you can just return undefined, null or, in fact, any falsy value in an event handler

does no this cover this purpose?

@rubendm92
Copy link
Contributor Author

Ok, so if I have a super simple event handler I could do

registerEventHandler('foo', () => ({}))
registerEventHandler('foo', () => null)
registerEventHandler('foo', () => effects.none())

Returning null looks fine to me in this case, but if the event handler returns multiple effects and one of them is conditional like this

registerEventHandler('foo', (_, {successEvent}) => ({
    ...some.effect(),
    ...(successEvent
      ? effects.dispatch(successEvent.id, successEvent.payload)
      : {}),
  })
)

Opens a console, checks if spreading null works and, yep, it works

Aaaand here you could use null too 😅 I was expecting null to fail with the spread operator, but it works so ok. If we want to go for any falsy value is ok for me to close this PR 👍

@mariosanchez
Copy link
Contributor

mariosanchez commented Oct 6, 2022

I wasn't covering this cases in my comment, but.. Ok xD If we think it's a good semantic addition, I'm okay with adding this anyway, since it's less dependent of knowing this particularity of JavaScript.

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.

4 participants