These are chat archives for canjs/canjs

2nd
Feb 2018
Kevin Phillips
@phillipskevin
Feb 02 2018 02:21
@RyanMilligan I don't think that plugin was ported to 3.0
you'll probably need to set up your own eventListener to get it to work in the capture phase
document.addEventListener("click", function() {}, true)
Viktor Busko
@Lighttree
Feb 02 2018 09:28

Guys, I have one question related life cycle hooks:

https://canjs.com/doc/can-component/connectedCallback.html

It looks not really obvious that connectedCallback returns disconnectedCallback, why not to have another life cycle hook named disconnectedCallback ?

Kevin Phillips
@phillipskevin
Feb 02 2018 14:35
connectedCallback just needs to return a function that tears down whatever was set up in the connectedCallback
if you had another lifecycle hook, you might have to store listeners somewhere
so that you could access them in the disconnectedCallback hook
that being said, I could see also having a disconnectedCallback
we will have that with can-element since customElements have a disconnectedCallback hook
Chasen Le Hara
@chasenlehara
Feb 02 2018 16:31
Watch our live contributors meeting in just a few minutes, where we’ll discuss the results of the survey and give demos of what we’ve been working on! https://www.youtube.com/watch?v=uBhUn9RJzXs
Gregg Roemhildt
@roemhildtg
Feb 02 2018 18:15
Gotta say, I finished the update to 4.0 and its not so bad. Props to you guys! Also I'm noticing a few really nice things, for one, functions aren't called automatically in stache, awesome! Also, I can see stache template tags and functions in the call stack now? That's pretty cool
RyanMilligan
@RyanMilligan
Feb 02 2018 18:16
@phillipskevin that's what we wound up doing, which is fine at the small scale we need it at, but would be an issue if we were having to do it in more places because of how easy it would be to leak those handlers. I still don't understand why the plugin doesn't work, though...I confirmed that it's running and Control.processors.capture is set, so why doesn't the callback even get called?
Kevin Phillips
@phillipskevin
Feb 02 2018 20:24
@RyanMilligan how is it running?
I mean, how did you install and load the plugin?
RyanMilligan
@RyanMilligan
Feb 02 2018 20:24
We imported the module at the top of the file that includes a "click capture" event.
And installed it with npm.
Kevin Phillips
@phillipskevin
Feb 02 2018 20:29
ok, I see the plugin now
so, that plugin is pulling in canjs 2.3
and modifying its can.Control
RyanMilligan
@RyanMilligan
Feb 02 2018 20:31
Ahhh, I see the problem. It's modifying the wrong Control.processors object. That's unfortunate, but at least it answers my question. Thanks.
Haha, so we should probably remove that plugin now that we're not using it anymore, huh? It's bringing in an entire old version of can that we don't want.
Kevin Phillips
@phillipskevin
Feb 02 2018 20:32
it shouldn't be that bad to update
try just copying that code into your app and changing import Control from 'can/control/'; to import Control from 'can-control';
and see if the rest of it works
@ilyavf would accept a PR I'm sure :smile:
RyanMilligan
@RyanMilligan
Feb 02 2018 20:44
Cool, I'll look into it. That really would be a nicer way to set it up than the manual work around we have now.

I'm actually having a bigger problem right now, though. We have a huge memory leak whenever we remove elements from the screen. It seems like basically the entire UI tree leaks every time. All the detached DOM trees I look at in the heap snapshots I've taken to try to diagnose it look basically like this:

HTMLTableRowElement -> ([1] -> [4] -> replacements -> masterNodeList) -> unbind -> context in teardown() -> (more DOM nodes) -> HTMLDocument

The stuff in parentheses varies when I look at different entries, but they all go through that "context in teardown()" thing. This appears to be the teardown() function in can-view-live/lib/core.js. This function is supposed to unsubscribe an event from the element that's passed to it, but it seems like it's causing both the event handlers and the elements themselves to leak.
Kevin Phillips
@phillipskevin
Feb 02 2018 20:50
I'm going to try to reproduce this today
RyanMilligan
@RyanMilligan
Feb 02 2018 20:51
I think it's likely, yes. In fact, in looking at it over the last hour or two, I realized that the error is being thrown from inside make-mutation-event/removeEventListener, which is also ultimately called by teardown().
Kevin Phillips
@phillipskevin
Feb 02 2018 20:52
ok
RyanMilligan
@RyanMilligan
Feb 02 2018 20:58
eventName always appears to be 'change' when the error is thrown, by the way. I tried removing all the places in our code where we subscribe to the 'change' event and it had no apparent effect.
And handler is undefined.
Kevin Phillips
@phillipskevin
Feb 02 2018 21:00
change events would be any time you do like value:bind
most can-stache-bindings will listen to change events
you said you weren't sure what it would take to create a minimal example that reproduces the problem?
RyanMilligan
@RyanMilligan
Feb 02 2018 21:02

Yes, that makes sense. I walked up the tree a little bit, and in observation[canSymbol.for(offValueSymbol)] there's a line:
var translationHandler = singleReference.getAndDelete(updater, this);

translationHandler is coming out undefined.

Right, I don't know what's different about our project from other ones that would cause it to happen. If a minimal application that creates and removes an element that has a two-way binding doesn't do it, then I don't know what the difference is in our project.
The particular text boxes I'm seeing on the stack when the error happens do use a converter for their value:bind property...maybe that's related.
RyanMilligan
@RyanMilligan
Feb 02 2018 21:09
I just removed the value:bind from those textboxes, and I'm still seeing the error and the leak.
Kevin Phillips
@phillipskevin
Feb 02 2018 21:12
can you give the markup for the text box?
RyanMilligan
@RyanMilligan
Feb 02 2018 21:13
<input type="text" class="input {{./layout.classes}}" value:bind="deref(./row, ./layout.name, ./layout@map, ./layout@unmap)" on:keyup="./layout.keyupFn(./row, ./layout, scope.element)" placeholder="{{./layout.placeholder}}" />
Kevin Phillips
@phillipskevin
Feb 02 2018 21:14
can you keep trying to remove things until it goes away? (if it will)
RyanMilligan
@RyanMilligan
Feb 02 2018 21:15
This is basically a generic form control that creates input items (in this case a textbox) for different fields, and layout is the object that defines how it should work. The keyup function actually essentially implements a two-way bind, so technically we could achieve this with value:to instead of bind. But I replaced the value:bind attribute with value=test and the error still happened with that textbox on the stack.
Kevin Phillips
@phillipskevin
Feb 02 2018 21:52
so @RyanMilligan that handler you were looking at is from https://github.com/canjs/can-stache-bindings/blob/v3.11.1/can-stache-bindings.js#L854
can you put a breakpoint there and figure out what prop is
RyanMilligan
@RyanMilligan
Feb 02 2018 22:31
Sure, give me a moment.
Kevin Phillips
@phillipskevin
Feb 02 2018 22:40
you may need to modify that file in our node_modules to store a reference to prop in that function
like var pr = prop;
otherwise chrome devtools might not show you what it is
RyanMilligan
@RyanMilligan
Feb 02 2018 22:41
Yup, already done. The first value is 'tooltipMessage', which is a property we bind using a custom helper in a from attribute.
So, tooltipMessage:from='helper stuff'. I'm testing now if it still gets there for that property without the helper.
Yes, it does. I changed it to just bind directly to a property and it still throws the error for that prop.
Kevin Phillips
@phillipskevin
Feb 02 2018 22:42
there error being that translationHandler is undefined?
RyanMilligan
@RyanMilligan
Feb 02 2018 22:43
Yes.
It's also inside a custom block helper (I forget the official term), so I'm removing that too to see if it matters.

Yes, it's still getting in there. At this point, here's the full line from the stache:

<span tooltipMessage:from="./layout@title" html="true">

Kevin Phillips
@phillipskevin
Feb 02 2018 22:45
ok, why are you setting tooltipMessage on that span?
RyanMilligan
@RyanMilligan
Feb 02 2018 22:46
Hrm, that's an excellent question. I'm not actually the person who wrote this template...let me look at it a moment.
It may have something to do with the block helper.
Kevin Phillips
@phillipskevin
Feb 02 2018 22:47
ok
i'm trying it out also
to see if that would cause a memory leak
RyanMilligan
@RyanMilligan
Feb 02 2018 22:48
Weird, it doesn't appear to do anything. That's the only place in the whole codebase 'tooltipMessage' shows up.
Unless it's in a node_module...maybe it's an attr?
Kevin Phillips
@phillipskevin
Feb 02 2018 22:48
git blame ?
RyanMilligan
@RyanMilligan
Feb 02 2018 22:50
I just removed it, we'll see what happens. The next one was "value", which is a bit trickier to track down exactly where it's coming from.
I searched the whole node_modules folder for 'tooltipMessage' and turned up nothing...I'm thinking it was probably something that went wrong in the migration from can 2.
Kevin Phillips
@phillipskevin
Feb 02 2018 22:51
you can also look at the el
to track down where the value is coming from
RyanMilligan
@RyanMilligan
Feb 02 2018 22:53
Yeah. So 'value' is from that first text input I talked about earlier. It's using :bind to a converter to set the value, but I previously changed it to a static string instead and still got the leak. Let me check if the error still happens.
The error is gone.
And the leak is smaller. 1.8 megs each time instead of 3-4.
Kevin Phillips
@phillipskevin
Feb 02 2018 22:56
ok... so value:bind="someConverter()" is causing (at least part of) the problem
RyanMilligan
@RyanMilligan
Feb 02 2018 22:56
Possible, I'm trying it now with from instead of bind.
It still happens with from.
And the leak grew back to 3.3 megs.
Same with to.
Kevin Phillips
@phillipskevin
Feb 02 2018 23:00
but value:bind="./row" (or something like that) made it go away?
RyanMilligan
@RyanMilligan
Feb 02 2018 23:00
I'm trying that now. Earlier I just set it to a constant string.
Kevin Phillips
@phillipskevin
Feb 02 2018 23:00
ok
RyanMilligan
@RyanMilligan
Feb 02 2018 23:01
Error still happens if I bind directly to a property, so the converter isn't the problem.
Kevin Phillips
@phillipskevin
Feb 02 2018 23:01
ok
and you said this is inside of a custom section helper?
like
{{#myHelper}}
  <input ...>
{{/myHelper}}
RyanMilligan
@RyanMilligan
Feb 02 2018 23:03
This one is inside of an {{#eq}} block, I can take it out if need be. We're only triggering one path here, I actually deleted the other one to simplify things.
The span that was weirdly setting a property that nothing seemed to look at was inside a custom section helper, and I deleted that whole thing. But the error still happened when I removed just the custom helper.
Kevin Phillips
@phillipskevin
Feb 02 2018 23:04
ok
I've gotta run in a minute. Do you have time Monday to screen share?
RyanMilligan
@RyanMilligan
Feb 02 2018 23:05
Yes, that won't be a problem.