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

Callback return value in custom event handlers #1553

Closed
PaulMaly opened this issue Jun 21, 2018 · 4 comments
Closed

Callback return value in custom event handlers #1553

PaulMaly opened this issue Jun 21, 2018 · 4 comments

Comments

@PaulMaly
Copy link
Contributor

PaulMaly commented Jun 21, 2018

For example, I want to write some custom event handler for form submission which will be additionally disable form fields to prevent modifying. So, I'll do something like this:

export default function(node, callback) {

   const submit = (e) => {
       e.preventDefault();
       // disable form fields
      const promise = callback(e);
      promise.finally(() => /* enable form fields */);
   };

   node.addEventListener('submit', submit);
   return {
      destroy() {
         node.removeEventListener('submit', submit);
      }
   };
}

And use it as follows:

<form on:sendForm="submit(event)">
    ....
</form>
<script>
    import sendForm from './events/sendForm.js';
    export default {
        events: { sendForm }
    };
</script>

It's very useful and I'll just use this custom event in all my forms which need this feature.

The problem, that right now callback handler looks like this:

console.log(callback);
/*
ƒ (event) {
    component.submit(event);
}
*/

So, I can't get a value returned by component's method.

I suppose, we could just add return statement to this generated code to get something like this:

console.log(callback);
/*
ƒ (event) {
    return component.submit(event);
}
*/

What do you think about it?

@Rich-Harris
Copy link
Member

It seems totally reasonable, I can't think of a reason not to do that off the top of my head. Any chance you could throw a PR together? It may not even need an additional test if it's already covered by the ones in test/js/samples.

@PaulMaly
Copy link
Contributor Author

@Rich-Harris Yes, I could provide a PR for that fix, but I didn't catch the meaning of the phrase "throw a PR together". Sorry about that, I'm not a native speaker, as you can see.)))

@PaulMaly
Copy link
Contributor Author

@Rich-Harris Check related PR, please!

@PaulMaly
Copy link
Contributor Author

Closed with #1557

Rich-Harris added a commit that referenced this issue Jun 23, 2018
Rich-Harris added a commit that referenced this issue Jun 23, 2018
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

No branches or pull requests

2 participants