Skip to content
This repository has been archived by the owner on Oct 29, 2019. It is now read-only.

Extend the isInstantiated method for a check against the old and new data-* value #32

Open
Inkdpixels opened this issue Sep 2, 2015 · 6 comments
Milestone

Comments

@Inkdpixels
Copy link
Contributor

This will come in handy if we implement the MutationObserver API and a script modifies the data-component-tag, which currently does not implies a re-run since the element was already instantiated(even though with a different name/constructor).

Maybe we could just transfer the this.elements array into an object which holds the key/value, or in our case the element/value, so we can check if the element should be instantiated with a different value.

Thoughts @reduct/owners ?

@Inkdpixels Inkdpixels added this to the 1.1.0 milestone Sep 2, 2015
@dfeyer
Copy link

dfeyer commented Aug 1, 2017

HI @Inkdpixels we face an issue the the Assembler, as the assembler keep track of the DOM element, if some JS replace those element, the event listener, memory, ... are not clean up by the JS garbage collector, because there is reference to the old DOM element. Not totally related to this issue, but I have the feeling that we can solve both of them ;)

@Inkdpixels
Copy link
Contributor Author

Hey @dfeyer I think this might be a separate issue, something like a destroy() method on assembler could help.

let app = assembler();

app.register(MyComponent);
app.register(YetAnotherComponent)

app.run();

setTimeout(() => {
    app.destroy(); // Removes all instances from the cache
    app.destroy('MyComponent'); // Removes all instances of the given component id from the cache
});

This might be easy to implement, since it only involves either partially or completely resetting two indices, namely this.components and this.elements which you can find here:
https://github.com/reduct/assembler/blob/master/src/assembler.js#L57-L67

I also thought of supporting the destroying of a single instance, but for this you would have to modify a bit more logic in the package, something like adding a data-_assemblerId="${randomUuid}" upon initialization might help, this way you as a user can tell the internal ID of the instance and could call app.destroy('MyComponent', myRandomUuid);.

What do you think? Could or would you like to give it a shot since you can also tell if it resolves your issue. Otherwise I would pick up on this issue in some days (most probably next week).

cc @grebaldi @akoenig

@akoenig
Copy link

akoenig commented Aug 3, 2017

@Inkdpixels Good approach! Just a naming nitpick: I would go with app.destroyComponent('MyComponent') – just to be explicit and state that there is only one component involved and not the whole app.

And another one: Should it be possible to register new components after app.destroy() has been executed? Maybe a philosophical question :D

@Inkdpixels
Copy link
Contributor Author

Good catch @akoenig - Regarding your question, currently it is possible to register new components after you've executed .run() since multiple .runs() are also possible, so ....

oi m8

@dfeyer
Copy link

dfeyer commented Aug 4, 2017

Currently i do some experiment to use the ObserverAPI to have some kind of garbage collector for "detached" component (remove the reference from elements, and remove the component). It start to work nicely. I send a PR as soon as possible

@dfeyer
Copy link

dfeyer commented Aug 4, 2017

Here a gist of my current solution: https://gist.github.com/dfeyer/04d1e95cfffa754600cbf43c59ed57f9

So if I replace some parts of the page, all the references to the old dom node are flushed (elements + components), so the listener are freed by the GC of the JS engine. And I can create new component manually on the newly create dom structure.

The next step should be to extend the use of the observer API and let the Assembler instanciate new components automatically.

Your comments on the gist are welcome ;)

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

No branches or pull requests

3 participants