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

Expose "cloning steps" to custom elements (cloneCallback) #176

Closed
hayatoito opened this issue Jul 6, 2015 · 12 comments
Closed

Expose "cloning steps" to custom elements (cloneCallback) #176

hayatoito opened this issue Jul 6, 2015 · 12 comments

Comments

@hayatoito
Copy link
Contributor

Title: [Custom]: Callback for cloneNode/importNode (bugzilla: 24570)

Migrated from: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24570


comment: 0
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24570#c0
Erik Arvidsson wrote on 2014-02-06 22:51:41 +0000.

Built in elements like inputs have hooks for cloning data that is not represented as children or attributes.

Strawman:

Add a cloneNodeCallback, which gets called after the element has been called, with a the original element as the context object and the new node as the only argument.

Then HTMLInputElement could be implemented as:

document.registerElemetn('input', {
prototype: {
proto: HTMLElement.prototype,
cloneNodeCallback: function(newNode) {
newNode.value = this.value;
}
}
});


comment: 1
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24570#c1
Anne wrote on 2014-02-07 13:59:04 +0000.

Are you also suggesting that browsers refactor their current implementation of to do this copying at the point this callback would normally happen? As currently it happens synchronously.

The difference would be observable if you have a mixture of custom and normal elements. A similar question applies to base URL changes in 24577. (I'm assuming when adopting we'll just change the node document of a custom element ourselves.)


comment: 2
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24570#c2
Anne wrote on 2014-02-10 11:15:26 +0000.

Note that this callback should be modeled after http://dom.spec.whatwg.org/#concept-node-clone-ext and take the same arguments. If that is not feasible for some reason I'd like to know why. We should try to refactor that callback to run just before the method call that caused the cloning to return so custom and normal elements have the same processing.


comment: 3
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24570#c3
Anne wrote on 2014-02-10 11:27:25 +0000.

Hmm, HTML's cloning steps for <script> cannot be done from a callback I think. Insertion has already happened at the point the callback is supposed to run and insertion triggers "prepare a script" which checks "already started" which is part of what is to be cloned.

So these hooks would not be good for <script>...


comment: 4
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24570#c4
Erik Arvidsson wrote on 2014-02-10 15:31:15 +0000.

I was in a rush when filing this. Conceptually I think we need to be able to explain how the built ins work in this area but I didn't think much about it at the time.

I agree that http://dom.spec.whatwg.org/#concept-node-clone-ext is the concept we want to achieve.

I'm not sure why you think "insertion has already happened". I think the order would be something more aligned to.

New Node - Created Callback
Old Node - Cloning Step Callback
...
New Node - Attached Callback (if ever inserted into a document)

I think the above would work for script. The cloning step would copy the "already started" flag and the attached callback would check if it set.

The semantics of this callback would be to call it as a last step of http://dom.spec.whatwg.org/#concept-node-clone and call the callbacks in document order in the cloned tree, passing the old node as the context object and the new node as the only argument (the document can be reached through ownerDocument if needed).


comment: 5
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24570#c5
Anne wrote on 2014-02-10 15:45:41 +0000.

You're right. I was confusing this with adoption.

It seems I can get rid of /document/. /clone children flag/ seems to be used by .

The callbacks needs to be queued as cloning is also used by range operations and we do not want to run script during those operations I think (but rather just before they return). Queuing them at the current point seems to take care of the recursion automatically.


comment: 6
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24570#c6
Erik Arvidsson wrote on 2014-02-10 16:27:46 +0000.

(In reply to Anne from comment #5)
Queuing them at the current point seems to take

care of the recursion automatically.

Yup, I believe we can use the same timing as we have for createElement/createdCallback.


comment: 7
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24570#c7
Jelte Liebrand wrote on 2014-08-04 23:10:01 +0000.

Hey guys,

Just wondering if this is still on the cards? I'm just running in to similar issues of not being able to hook in to the cloneNode, so curious to see if this will get resolved in the near future or if I should continue to jump through some hacky loops?

/Cheers,
Jelte


comment: 8
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24570#c8
Anne wrote on 2014-08-05 08:08:24 +0000.

Well, we'll need to sort this out one way or another if we want "Blink-in-JS" and custom elements in general to succeed. So yes.


comment: 9
comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24570#c9
Adam Klein wrote on 2014-11-24 18:26:12 +0000.

*** Bug 27420 has been marked as a duplicate of this bug. ***

@dominiccooney
Copy link
Contributor

This came up in TPAC 2016 Web Components/Mon discussion.

If I recall correctly Google and Mozilla expressed a preference for tree order, and there was no dissent. Microsoft and Apple were present but didn't state a preference. Here are the minutes:

bz: do we want to run them in tree order? before or after its kids?

dcooney: callbacks run after everything is cloned
... the simplest way would be parents before children

bz: might want to discuss with components authors

Domenic: motivation is emulating builtins
... those don't depend on children

koji: can those callbacks mutate the tree?

Domenic: yes, but it's well-defined
... I'd like to add them in tree order

annevk: like

chaals: like

dcooney: we could get some more feedback and decide Friday
... just to double check

Domenic: SGTM

weinig: I don't think we're against it, sounds fine
... there's a question of how often it's used

bz: it would help to have actual use cases

Domenic: if HTML modules takes off, you really need cloning
... so need to get some more feedback
... but generally sounds acceptable

I followed up with authors using web components at Google and they preferred tree order too. Their rationale was it is consistent with other callbacks in the platform.

@domenic Can you add this callback, passing the original element?

What should this callback do if the clone is being created in a document without a browsing context? The element being cloned may be a defined, custom element that has been moved into a document without a browsing context, because elements hang onto their definitions. But the document may not have a browsing context, so the clone would not normally be upgraded in that case. Maybe the callback should not fire in that case.

There's a similar question of what the callback does if the clone is being created in a document with a different window hence a different custom element registry. That registry may or may not have a matching definition. Maybe the callback should fire in that case? It would let an author who registers the same definition in every window in their app also pass state when cloning. The callback would have to check the defined pseudoselector, prototype, etc. to determine what to do with the clone. (In which case, cloning into documents without a browsing context could call the callback anyway for consistency.)

/cc @rniwa @bzbarsky

@rniwa
Copy link
Collaborator

rniwa commented Sep 22, 2016

Could someone tell me what the semantics of the proposed clonedCallback would be?

@dominiccooney
Copy link
Contributor

I'll take a stab at it...

cloneNode, insert a step after 2.1 which does something like: if node is a defined custom element, enque a custom element clone reaction in copy's queue with node, definition, copy.

This idea of enqueueing in a random element's queue (it may not have a definition!) is new, but it will be useful if we investigate the "custom attributes/global attributes" thing. The reason you want to use that queue, and not the queue of the source node being cloned, is because the queues are there to deliver callbacks in a consistent order from the element-centric point of view the custom element author has. If you put the clone callback in the queue of the source node being cloned, convenient as it is, it will mean the cloned data will arrive at a "random" point of time from the point of view of the copy.

The cloned callback reaction will invoke something called clonedCallback from the source node's definition; this is source node; there's one argument: copy.

@dominiccooney
Copy link
Contributor

Hmm, I'm not sure about that copy's queue part. There are two elements involved so it is not clear which queue to use. Maybe you enqueue a reaction in each queue, and the first one to run cancels the other.

We always have edge cases if an author's custom element callback reaches out and messes with the DOM. However this callback is a bit different because we always thought it was OK for an author to change the element's state. So crafty queue handling is trying to preserve that idea.

@rniwa
Copy link
Collaborator

rniwa commented Sep 22, 2016

I don't want to schedule on both elements. I think calling this callback on the cloned element provides a slightly better ergonomics because you'd be then copying states from the source like this:

clonedCallback(source) {
    this._someState = source._someState
}

@domenic
Copy link
Collaborator

domenic commented Sep 22, 2016

Some misc points:

  • If the argument is source instead of dest, then the name should be something like clonedToCallback.
  • I wonder if we want to just not call this unless both elements have the same definition? It seems unlikely that people would have a useful behavior for arbitrary elements. E.g. @rniwa's above code is pretty buggy if source is not an element with the same definition as this.
  • We could make this a static callback, i.e. static clonedCallback(source, dest) { ... }. I think this makes more sense if we go with my above idea of only calling it when the definitions are the same.

Apologies if any of this is totally off; I'm not feeling 100% this morning.

@dominiccooney
Copy link
Contributor

These are the questions we need to answer:

  • Is the method "static"; or if it is an "instance" method is the receiver the source element or the new element?
  • Which queue(s) facilitate the callback timing?
  • What is the callback named?
  • What are the conditions to cause the callback to be called/skipped?

@rniwa By scheduling in both queues, I am not intending to have two callbacks; I was suggesting the queue that reached the callback first would run it and nullify the other one.

Here's the tough part of when to run the callback: You want to pull the state from the source element before it processes any more callbacks, which could change its state; this is something controlled by the source element's queue. You want to push the state to the new element after its constructor and attribute sets and not before; this is something that depends on the new element.

@domenic It would be a shame to break transmitting state when cloning in a new document if you could, in fact, register some definition in the new document. INPUT can do this; why can't the author do this?

What do you think about taking the definition from the source node and transmitting it to the new node? That simplifies things a bunch; if you had a definition before, the new node has the same definition.

@rniwa
Copy link
Collaborator

rniwa commented Sep 22, 2016

@rniwa By scheduling in both queues, I am not intending to have two callbacks; I was suggesting the queue that reached the callback first would run it and nullify the other one.

I know what you were suggesting, and I'm against adding such a race condition.

@dominiccooney
Copy link
Contributor

It's not a race in the sense of a concurrent race. What do you think about splitting it into two callbacks--externalize state and set state?

@domenic
Copy link
Collaborator

domenic commented Sep 22, 2016

It would be a shame to break transmitting state when cloning in a new document if you could, in fact, register some definition in the new document. INPUT can do this; why can't the author do this?

Well, <input> is "pre-registered with the same definition" in all documents. But the idea of doing registration in the other document during the cloning callback is very interesting. A bit uncool to modify global state like that, but it might be just what you want in some cases.

What do you think about taking the definition from the source node and transmitting it to the new node? That simplifies things a bunch; if you had a definition before, the new node has the same definition.

This is also a very interesting idea. I like it, on the surface; still working through the implications.

@annevk annevk changed the title [Custom]: Callback for cloneNode/importNode (bugzilla: 24570) [Custom]: Callback for cloneNode/importNode Sep 6, 2017
@annevk annevk changed the title [Custom]: Callback for cloneNode/importNode Expose "cloning steps" to custom elements (cloneCallback) Mar 5, 2018
@annevk
Copy link
Collaborator

annevk commented Mar 5, 2018

There doesn't seem to be much appetite from implementers. Needs more compelling use cases to be reconsidered.

The lack of parity with built-in elements was considered (e.g., script and input), but we're not convinced those are not mistakes (apart from template, which we're okay with being a special snowflake).

@trusktr
Copy link
Contributor

trusktr commented Feb 25, 2019

I was considering trying domc out (one of the fastest libraries in the Krause JS Framework Benchmark due in part to use of cloneNode), but my fear is that domc's use of cloneNode might cause errors or bugs due to missing JS properties in cloned custom elements.

Maybe Custom Elements could use something like clonedOutputCallback and clonedInputCallback: clonedOutputCallback would return something (f.e. an object containing state) from a custom element being cloned, and clonedInputCallback on the new custom element receives the output of the original node's clonedOutputCallback in order to place that state back where it belongs inside the newly cloned custom element?

EDIT: What if like in other libraries, clonedCallback or whatever it is called, returns the new object?

class Foo extends HTMLElement {
  clonedCallback() {
    // author decides how to make clones.
    const clone = new Foo()
    clone.copy(this)
    return clone
  }
}

The popular library Three.js implements .clone() methods on many of it's classes; it's a common pattern there. The clonedOutputCallback/clonedInputCallback idea would be better if the browser needs to be in control of construction. The browser could also just check that the prototype chains match, and throw if they don't.

EDIT 2: seems like this would be a more used feature compared to adoptedCallback. I've never used document.importNode() in my life, and not sure I ever will, but I could definitely see the convenience of cloning nodes, so it seems like it'd be nice to have cloned...Callback.

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

No branches or pull requests

6 participants