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

dispatch: modify dispatch to accept Event object instead of eventType and options #9

Closed
amarajs opened this issue Jul 10, 2018 · 4 comments
Labels

Comments

@amarajs
Copy link
Collaborator

amarajs commented Jul 10, 2018

Hello again.

Any chance you can modify dispatch to accept an Event object directly? The idea is to dispatch that Event if provided, or else fall back to the current functionality of creating a new CustomEvent for the developer.

I'd like to create event "envelopes" that team members can use to provide variable information in an expected format to ancestor event handlers.

For example, here's what I'd like to do:

import { actionCreator, actionEvent } from 'shared/actions';

function buttonClicked(host) {
    const event = actionEvent(actionCreator(/* possible data here */));
    dispatch(host, event);
}

const MyComponent = {
    render: () => html`
        <button onclick="${buttonClicked}">click me</button>
    `
}

The alternative is calling dispatchEvent manually, which might confuse some developers as it requires writing different code for what is fundamentally the same concept:

function buttonClicked(host) {
    const event = actionEvent(actionCreator(/* possible data here */));
    host.dispatchEvent(event); // use built-in EventTarget method
    // but this inconsistency might confuse other developers
}

Or, even worse:

function buttonClicked(host) {
    const action = actionCreator(/* possible data here */);
    dispatch(host, 'action-event', {
        bubbles: true,
        composed: true,
        cancelable: true,
        detail: action
    });
    // has to be written every time
}

I know it's a minor request, but it's one that could provide some nice conformity to end users.

@smalluban
Copy link
Contributor

smalluban commented Jul 11, 2018

The main reason why I've added dispatch helper function into the library was to simplify the process of dispatching events. Because of that, I wanted this function to be as simple as possible - just passing name of event and possible options.

I am not sure what your actionCreator(/* possible data here */) returns to actionEvent, but the latter returns CustomEvent instance, right?

You can achieve a similar effect without modifying or extending dispatch function. If your actionEvent would return ['eventType', options = {}], you could use it like this:

const dispatchArgs = actionEvent(actionCreator(/* possible data here */));
dispatch(host, ...dispatchArgs);

In this way, your actionEvent does not have to create the custom event directly anymore and you still using dispatch from hybrids.

What do you think?

@amarajs
Copy link
Collaborator Author

amarajs commented Jul 11, 2018

Unfortunately, I can't rely on developers of all skill levels understanding dispatch(host, ...args) syntax. But I could provide a method that creates the init options object, and rely on all developers providing the event name manually:

dispatch(host, 'action-event', actionCreator(/* data here */))

This has the benefit of matching the existing dispatch arguments in Hyrbids. The downside is there's a bit more boilerplate for developers to write.

@amarajs amarajs closed this as completed Jul 11, 2018
@smalluban
Copy link
Contributor

smalluban commented Jul 11, 2018

After all you can also hide implementation details into the actionCreator, which could take host as an argument and dispatch event for that host:

dispatchAction(host, /* data here */); 

// and inside of the dispatchAction
function dispatchAction(host, /* data here */) {
  ...
  dispatch(host, 'action-event', { ..., detail: action });
}

@amarajs
Copy link
Collaborator Author

amarajs commented Jul 11, 2018

That's a great idea, also! Thank you.

@smalluban smalluban changed the title Request: modify dispatch to accept Event object instead of eventType and options dispatch: modify dispatch to accept Event object instead of eventType and options Nov 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants