Coder Social home page Coder Social logo

Comments (19)

WebReflection avatar WebReflection commented on June 16, 2024

is innerHTML involved anywhere in your case?

from document-register-element.

shawnbot avatar shawnbot commented on June 16, 2024

Nope, no innerHTML here.

from document-register-element.

WebReflection avatar WebReflection commented on June 16, 2024

is this an IE8 related issue? in that case I can certainly confirm the creation might be asynchronous.

In other words, I really need way more details about this issue or it's kinda a blind guess to fix it.

Thanks for any sort of extra collaboration.

from document-register-element.

WebReflection avatar WebReflection commented on June 16, 2024

P.S. the constructor you are saying works already does exactly the same thing of creating a registered element

We have MutationObserver or DOMNodeInserted fallbacks and on IE8 I think some timer beside onpropertychange or similar. All these methods are asynchronous and will/should trigger the createdCallback as soon as possible but there's no guarantee it's going to happen synchronously.

This is why tests are often asynchronous too: https://github.com/WebReflection/document-register-element/blob/master/test/document-register-element.js#L113-L128

Accordingly, if the question is: do elements get created asynchronously? The answer is yes or maybe and it won't get fix or change for any reason otherwise performance of every application based on this polyfill would be compromised with a blocking logic that makes little sense.

Does any of my extra details answer your question and case?

from document-register-element.

shawnbot avatar shawnbot commented on June 16, 2024

This is not IE8-specific, and appears to affect all of the browsers I've been testing that use the polyfill (that is, everything except Chrome, but namely Firefox 40 and IE11). The constructor you referenced above is super interesting, though, and I'm wondering if you have any idea how using the constructor rather than document.createElement() might have an impact on when (or whether) the instance gets "shimmed" with the custom element prototype.

In my case, the custom element definition for eiti-bar is here, and in some cases hundreds of them get created in a single frame via D3 here. That snipped used to be just .append('eiti-bar'), but when I set attributes on those elements (also in the same frame) neither Firefox nor IE11 was calling the attribute changed callback. I tried setting the properties directly instead, but the property descriptor setters weren't being called either, which suggests that the custom element prototype wasn't being applied synchronously. So now that I know that it is, I'm even more confused as to why using the constructor does work.

from document-register-element.

WebReflection avatar WebReflection commented on June 16, 2024

to be honest I'd like to see a demo page with the issue, all I see right now is a potential infinite loop based on the fact that you are setting directly properties from attributes instead of using this.getAttribute('min') or max or value.

The attributeChanged function as well as the attachedCallback and the attributeChangedCallback look like completely redundant while min, max and value that should reflect their attribute in the node should be getters that return numericProperty(this.getAttribute('min')) or others so that you don't even need an initialization in order to make your node work and you are free from side-effects whenever a polyfill would invoke attribute changes whenever you also set node.prop = value also because when you se tthose properties you don't use numericProperty normalizer.

So I see some possible bug, some redundant logic, but I've no way to test your specific case.

from document-register-element.

shawnbot avatar shawnbot commented on June 16, 2024

Okay, first off: I might be a little bit crazy. I'm pretty sure that Firefox wasn't working yesterday, but now I switched back to using document.createElement() instead of the constructor, and it works. And I'm switching to using the properties because they're faster. (Aside: I'm curious how you suggest doing reflection generally, though.)

What's strange, however, is that in Firefox when I synchronously log element instanceof EITIBar I see false, but on the next frame it logs true. IE11 still appears not to apply the property descriptors, either sync or async. The setters don't fire when I set properties on new or old instances when I use document.createElement(), but do when I use the constructor. WTF?

from document-register-element.

shawnbot avatar shawnbot commented on June 16, 2024

I'm going to make a standalone test case for this next.

from document-register-element.

WebReflection avatar WebReflection commented on June 16, 2024

Have you forgot I said it is most likely asynchronous when it gets created?

You don't want to use instanceof before craetedCallback has been invoked. Nodes are promoted at setup time and usable after createdCallback happens. This should not be your concern anyway so please create this test case.

My suggestion wasn't about performance but consistent data. You are normalizing default max min and value on the prototype but you don't normalize these values when somebody use setAttribute.

Also please note attributes will trigger attributeChangedCallback only if you change them via setAttribute, which is why I am suggesting to have getters and eventually throw or update via setAttribute if you try to set min max and value so you are sure you reflect one value either ways.

from document-register-element.

shawnbot avatar shawnbot commented on June 16, 2024

I think I read your async comment above to say that it was IE8-specific, but after reading it again it sounds like you're saying it's not.

Either way, what I'm seeing in this test is that setting the properties inside the attachedCallback() doesn't fire the setters, even in Chrome's native implementation. Wrapping the attached logic in requestAnimationFrame() fixes that, but I don't understand why. In Chrome, the instance has custom element methods, but it looks like the property descriptors aren't there until the next frame. Can that be right?

from document-register-element.

shawnbot avatar shawnbot commented on June 16, 2024

And I guess my follow-up question is: If application of the custom prototype is async, what's the best way to know when it's finished so we can render as soon as possible?

from document-register-element.

WebReflection avatar WebReflection commented on June 16, 2024

You don't render CustomElements, you should not interfere with the way nodes are rendered.

The registering is synchronous but the rendering is asynchronous, which is why exists a createdCallback in the first place.

Your demo shows same result in Chrome and Firefox and I don't know what you are trying to do. If you are trying to react to properties directly changed like node.value = 20 you are somehow mistaking how CustomElements work and you should really opt for getters and setters on that value or other properties.

How would it look like? Like the following:

document.registerElement('my-el', {
  prototype: Object.create(
    HTMLElement.prototype,
    {
      max: {
        get: function () {
          return this.getAttribute('max');
        },
        set: function (value) {
          this.setAttribute('max', value);
        }
      },
      min: {
        get: function () {
          return this.getAttribute('min');
        },
        set: function (value) {
          this.setAttribute('min', value);
        }
      },
      value: {
        get: function () {
          return this.getAttribute('value');
        },
        set: function (value) {
          this.setAttribute('value', value);
        }
      }
    }
  )
});

In such way you don't have to do a thing about atributeChanged because everything is going to be fine. You might want to return numericProperty within the getter OR you might want to save computed data in another way within the setter.

In any of my code there is a concurrent possibly messed up double way to interact with a node attribute that would make your data inconsistent.

Is this part clear? 'cause I believe your entire logic is based on node.value = 123 scenario which is not what CustomElement specification is about when it comes to atributeChangedCallback behavior.

from document-register-element.

shawnbot avatar shawnbot commented on June 16, 2024

Well, the whole point of the attribute reflection was to allow the setting of properties via attributes in addition to the accessors, and lazily render when either is changed. That is, I want to be able to drop <eiti-bar max="100" value="50"></eiti-bar> into my document and it will render without having to interact with it via JS.

I think what's going on is actually much more interesting: if the element hasn't been "initialized", then the property in question gets set directly (not via the property setter), and then the accessors never work because they're part of the prototype. In other words, element.hasOwnProperty('value') === true, which should not be the case because it's an inherited property.

from document-register-element.

WebReflection avatar WebReflection commented on June 16, 2024

you setup that on createCallback ... I'll try to explain again: Custom Elements invoke the attributeChangedCallback only if you use setAttribute, and only if such attribute is different.

In order to have a reliable copy of these properties you need to use getters and setters so that attributeChangedCallback will be invoked either if you node.setAttribute('value', 80) or you node.value = 80, reflecting somehow what an input element would do.

In both cases you are indirectly triggering the attributeChangedCallback which is what you want to do.

The initial setup is at this point redundant, you can simply render right away.

If performance is a concern while getting properties you can define a this._data = {} in the creationCallback and set this._data.value = normalize(whatever) within the setter or the atributeChangedCallback so that the getter for that value would be straight away this._data.value

I still don't see a simple example that reproduces the problem though, so I'm not sure how to move on here.

from document-register-element.

trusktr avatar trusktr commented on June 16, 2024

I've noticed that under some circumstances (I'm still working on a test case), the polyfill can behave differently depending on whether you create an element via document.registerElement() and the class constructor.

I think you meant document.createElement() here. When you use document.registerElement(), that returns a constructor. Using that returned constructor to instantiate an element should theoretically be the same as using document.createElement() to instantiate the same element. However, if you supply a class to document.registerElement(), as in

class MyElement extends HTMLElement {
  constructor() { /* ... */ }
}
document.registerElement('my-element', MyElement)

then doing

let el = new MyElement()

will not be the same as document.createElement() because the document.registerElement() call replaces the supplied constructor function with its own. For it to work as expected, you'd need to change the definition to this:

class MyElement extends HTMLElement {
  constructor() { /* ... */ }
}
MyElement = document.registerElement('my-element', MyElement)
// ^ Here, assign the new returned constructor to MyElement.

At this point, now using the constructor or document.createElement() is the same.

In my humble opinion, the Custom Element spec should be modified so that the original class constructor can be used without having to do that assignment trick. I'm sure there's a way to make that possible... The problem stems from the fact that the document.registerElement() API was designed before ES6 classes came around, and so I don't think that was taken into consideration (I could be wrong, but that's my guess). The reason I believe so is because many of the examples don't use ES6 classes, like for example from html5rocks:

// ES5-style extension:
var XFooProto = Object.create(HTMLElement.prototype);
...

var XFooExtended = document.registerElement('x-foo-extended', {
  prototype: XFooProto,
  extends: 'x-foo'
});

from document-register-element.

trusktr avatar trusktr commented on June 16, 2024

@shawnbot Well, anyway, I don't even know if you were using ES6 classes. As for the attribute issue, I don't seem to have a problem like that. Got a code sample? EDIT: nvm, I see the eiti-bar code, checking it out... By the way, here's what it looks like to define an element using ES6 classes, which I think is much cleaner: https://github.com/infamous/infamous/blob/master/src/motor-html/node.js#L33-L193

from document-register-element.

trusktr avatar trusktr commented on June 16, 2024

@shawnbot At first glance, I don't see what's wrong. You shouldn't need requestAnimationFrame like that. Can you simplify the example and make the render function simpler just for testing (like just add a border or something, or just log some values).

from document-register-element.

WebReflection avatar WebReflection commented on June 16, 2024

this got stuck so I'll close it.

from document-register-element.

shawnbot avatar shawnbot commented on June 16, 2024

Sorry for the confusion, everyone! Let me try this again:

My custom element represents a single bar in a (presumably multi-bar) chart, and the size and position of the bar is a function of its min, max, and value properties. These are reflected (setting the attribute sets the corresponding property, and vice-versa) so that other DOM-aware tools don't need to know whether to set an attribute or property, and so that we can target certain types of bars in CSS (e.g. those with negative values: [value^="-"]).

The issue I was running into (as I explained here) was that, for some reason, the property descriptors for min, max, and value weren't available in the createdCallback(), which meant that setting those properties in that function sets each instance's "own" properties rather than firing the setters. I think that this was a side effect of JSFiddle executing the JavaScript on window load, which meant that setting properties on the elements created with document.createElement() was in effect overriding the property descriptors. From that point on, there was no way to access the getters or setters.

Anyway, I went back and started over, and the result works great, so it must've been a problem in my original implementation. My only real hack in this one was to use HTMLElement.prototype.setAttribute(this, name, value) inside the property setter, which I assume avoids recursive calls to the attributeChangedCallback(). This appears to be the case in both the polyfill and Chrome's native implementation, but I'm curious if y'all think this is the right way of doing property/attribute reflection.

from document-register-element.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.