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

can.Component events registered twice in 2.2.x #1778

Closed
gsmeets opened this issue Jul 11, 2015 · 4 comments
Closed

can.Component events registered twice in 2.2.x #1778

gsmeets opened this issue Jul 11, 2015 · 4 comments
Labels
Milestone

Comments

@gsmeets
Copy link
Contributor

gsmeets commented Jul 11, 2015

I've made a fiddle using the code below. I'm using a custom processor here, just to be able to hook into the click handler. In my private setting I've tested it by adding a log message to the basic processor in can.control.

If you run the fiddle you can see in the console that the click processor is called twice.
The event is not triggered twice, so it's relatively harmless, but there seems to be something superfluous going on. I wonder if this has any performance impact.

This problem was introduced in 2.2.0, 2.1.4 is working as expected.

http://jsfiddle.net/gsmeets/8xq899vc/3/

can.Control.processors.click = function( el, ev, selector, methodName, control ) {
    console.log( arguments )
    return $.noop;
};

can.Component.extend({
    tag: "event-fired-twice",
    template: "<p>test2</p>" ,

    events: {
        "click" : function () { }
    }
});

var renderer = can.stache( "<event-fired-twice>test</event-fired-twice>" );
$( "body" ).append( renderer());
@gsmeets gsmeets changed the title can.Compnent events registered twice in 2.2.x can.Component events registered twice in 2.2.x Jul 11, 2015
@alexisabril
Copy link
Contributor

This is due to the fix provided for: #1422

Specifically the re-binding on line: https://github.com/bitovi/canjs/blob/c3b340422e4b3584b44e9f6f9940331c6b1e6d94/component/component.js#L272

Unsure what the impact would be of having a conditional rebind vs just rebinding.

alexisabril added a commit that referenced this issue Jul 15, 2015
@daffl daffl modified the milestones: 2.2.7, 2.2.8 Jul 23, 2015
@justinbmeyer
Copy link
Contributor

I'm betting this is fixed by #1803. cc @daffl

@alexisabril
Copy link
Contributor

Confirmed, this is fixed by #1803. PR: #1809 added just for the test. Note: test passes in master, however it looks like there's some merging going on. Current PR is for minor.

@alexisabril
Copy link
Contributor

Closing as this is fixed in master, #1809 is open to track progress of the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants