ractivejs / ractive

Next-generation DOM manipulation
http://ractive.js.org
MIT License
5.94k stars 396 forks source link

.hasListener() detects listeners erroneously #3253

Closed ceremcem closed 6 years ago

ceremcem commented 6 years ago

Description:

Even we don't supply a real listener, ctx.hasListener('mylistener', true) considers the mylistener as if it was a real listener:

image

Versions affected:

0.10.4

Reproduction:

Playground

evs-chris commented 6 years ago

I think I may be misunderstanding this one:

There's a component, mycomp, which is checking for an on-hello listener. This component is used in the main template both with and without an on-hello, and it is detected correctly. There's also a mysupercomp component that has a <mycomp on-hello="hello">, which is a mycomp with a hello listener, so should't all mysupercomp instances have their mycomp detect an on-hello listener?

ceremcem commented 6 years ago

From the point of view of mycomp instances which are inside mysupercomp, every instance naturally would have a hardcoded hello handler (by definition). But if we look from the point of view of functionality, as mycomp decides its behaviour by looking at hello handler definition, we should be able to use same behaviour of mycomp while using in another component.

My current workaround is as follows: I'm checking if context.hasListener('hello') in the parent component (in mysupercomp) and pass the handler to the mycomp conditionally: Playground.

I think this behavior should be the default one.

evs-chris commented 6 years ago

After pondering this for a bit, it's still a bit chewy. The inner component can't really check for an on-hello above it's own template, because the corresponding event is no longer hello beyond its own template because at that point it's mycomp.hello. I think the best solution to this would be to add events to the set of component registered attributes in some way so that they can get yielded through correctly. That would let the mysupercomp pass its on-hello through to mycomp, where it could then be detected correctly, though I'm not 100% sure how that works out with crossing contexts.

ceremcem commented 6 years ago

So can't a component pass any listener to the child only if that listener exists? It can't be that easy, but my first attempt is using a regex for this purpose:

template.replace(/(\s+)(on-[a-z-]+)=(["'])([a-zA-Z0-9]+)(["'])/g, 
    '$1{{#if @this.getContext().hasListener(\'$4\')}}$2=$3$4$5{{/if}}')

Playground

ceremcem commented 6 years ago

...yeah, moreover, there is an extra complexity in the process. If we take a look at this example, we have to define a MITM event which would mislead the regex:

Ractive.components["super-foo"] = Ractive.extend({
  template: `
    <foo y="superrr" 
      {{#if @context.hasListener('select',true)}}on-select="_select"{{/if}}
      >
      {{#partial custom}}
        button count {{x}}
      {{/partial}}
    </foo>
  `,
  on: {
    _select: function(ctx, ...args){
      var c = ctx.getParent(true); c.refire = true;
      this.fire('select', c, ...args);
    }
  }
})

Here _select is not meant to be declared if select listener does not exist for super-foo component. Thus, my previous regex won't work for this case as it will always detect _select listener declared.

There are 3 different handlers here: select, select and _select (yes, first two shares the same name). First select is what super-foo listens for, eg <super-foo on-select="blahblah" ..., second select is what foo expects to listen, third one, the _select is actually the first one with MITM applied to re-propagate the foo context.

I accept that this is something we should handle manually. Less magic is better, that's why I'm going to close this issue.