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

this has wrong meaning in custom event handler #1297

Closed
Rich-Harris opened this issue Mar 31, 2018 · 2 comments · Fixed by #1376
Closed

this has wrong meaning in custom event handler #1297

Rich-Harris opened this issue Mar 31, 2018 · 2 comments · Fixed by #1376
Labels

Comments

@Rich-Harris
Copy link
Member

In this situation (REPL)...

<p>Select the input, hit 'enter', check the console</p>
<input on:enter='this.blur()'>

<script>
  export default {
    events: {
      enter(node, callback) {
        function handleKeydown(event) {
          if (event.which === 13) callback();
        }

        node.addEventListener('keydown', handleKeydown);

        return {
          destroy() {
            node.removeEventListener('keydown', handleKeydown);
          }
        };
      }
    }
  };
</script>

I'd expect this.blur() to refer to the <input>, but this code is generated:

enter_handler = enter.call(component, input, function(event) {
	this.blur();
});

this should probably be rewritten as input.

Workaround is this:

-if (event.which === 13) callback();
+if (event.which === 13) callback.call(node);
@ekhaled
Copy link
Contributor

ekhaled commented Apr 5, 2018

I have a couple things to point out.

  1. Would fixing this break BC
  2. Will fixing this mean we cannot access the component context anymore from inside the handlers
  3. The new actions have a similar signature, would they benefit from being called in the context of the node

@Rich-Harris
Copy link
Member Author

Good news all round:

  1. No, because at the moment this is undefined, so you can't use it anyway. It's really just a bugfix
  2. the only this that's affected is the handler itself — this.blur() becomes input.blur(). The enter function is still called with the component as context
  3. Actions receive the node as an argument and have the component as this (same as event handlers) — I think that should stay the way it is

Rich-Harris added a commit that referenced this issue Apr 29, 2018
overwrite this in custom event handlers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants