Webcomponents: Polymer encouraged incorrect document.createElement() usage

Created on 11 Aug 2016  ·  39Comments  ·  Source: WICG/webcomponents

See https://bugzilla.mozilla.org/show_bug.cgi?id=1294100 and references from there. It seems like we need to support document.createElement(string, string). Not entirely sure if ignoring the argument or treating it as id is better.

Sigh.

custom-elements v1

Most helpful comment

I think we should just use the new syntax and not introduce any new complexity into the spec. Our plan is as follows:

  • Blink will start warning when the old (v0) syntax is used, along with the rest of v0 deprecation. We might even start warning sooner.
  • Polymer will update the v0 polyfill to accept the new syntax, and create a warning if someone is using the old syntax. The next version of Polymer and v1 polyfill will only allow the new syntax.
  • We'll update Polymer/html5rocks/webcomponents.org docs to only talk about the v1 syntax.

All 39 comments

@dominiccooney, have we seen fallout from this in Blink?

Ugh.

I'd definitely prefer ignoring the argument; we should not opt in old code to the new semantics...

@smaug---- was saying the same.

My 2cents: document-register-element now supports 3 document.createElement overloads: (string), (string, stringIs), (string, object{is:string})

After all I don't see why the is case should fail in polyfilled browsers, sent either as string, like it was already in v0 specs, or object with an is property.

The reason it should fail (ideally) or do nothing silently (as is probably what we'll have to spec) is that the semantics have changed. It is the same reason we had to rename all the APIs. We cannot have old v0 code transparently upgraded to v1 semantics, since they are different.

So, if I understand correctly, passing an object should be V1 only and passing a string V0 only?

Yes (in browsers that implement v0, i.e. Chrome only).

Thanks, mine right now is a polyfill for v0 but I'm planning to bring in V1 too.

Although I thought WebKit didn't agree about the is property, did they change their mind? If the V1 is final and agreed by all vendors then it's time to polyfill it.

WebKit does not agree with customized built-in elements, but they are included in the standard since we have multiple vendors interested in implementing. If it turns out that for some reason that doesn't happen, they will be removed.

Nothing in spec land is ever final, however. It's a judgment call when you want to start implementing something. All major web browsers already have.

I understand nothing in spec is never final but V0 somehow is, and V1 hopefully is as well.
Versioning is a common "_nothing ever final_" approach 'cause vendors/implementors need to actually implement something so ... is V1 still heavily discussed or something worth polyfill?

I'm just trying to have an ETA for my poly users and myself too. Thanks

v0 is abandoned, not final. v1 is still being heavily discussed as it is being implemented.

Like we've stated multiple times in the past, we're not going to implement custom builtin elements in WebKit but all other aspects of custom elements should be interoperable across browsers.

I guess that would be another way for Fx to dodge this. Just not implement custom builtins…

Blink implements an old draft of the custom elements spec where that option is a DOMString. Our implementation of custom elements "v1" and that old stuff are basically separate. We won't support interpreting that second argument as a string when creating customized built-in elements in V1 and will deprecate that along with document.registerElement and the rest of the old implementation.

Blink implements an old draft of the custom elements spec where that option is a DOMString

Looks like that's here: https://www.w3.org/TR/2016/WD-custom-elements-20160226/#extensions-to-document-interface-to-instantiate

Also it looks like versions of react.js used the stringy 2nd argument until fairly recently. See https://github.com/facebook/react/pull/6896 for discussion and related links.

Seems like a lot of deployed code requiring the old custom elements overload behavior...

Since still open, and just FYI, my alternative Custom Elements only polyfill implements both cases accordingly if you defined through V0 or V1. If in V0 it accepts an optional string for the is="...", if in V1 it accepts an object like {is: "button"}.

I think we should just use the new syntax and not introduce any new complexity into the spec. Our plan is as follows:

  • Blink will start warning when the old (v0) syntax is used, along with the rest of v0 deprecation. We might even start warning sooner.
  • Polymer will update the v0 polyfill to accept the new syntax, and create a warning if someone is using the old syntax. The next version of Polymer and v1 polyfill will only allow the new syntax.
  • We'll update Polymer/html5rocks/webcomponents.org docs to only talk about the v1 syntax.

atm browsers are forced to support also the old syntax, if they support also the new syntax. If only new syntax is supported, browsers throw when passing string as the second param.

@smaug---- right. I am proposing that we only support the new syntax and throw. There's a simple workaround for the error (literally a few characters' worth), and we (Blink) will provide the ramp-down for the old usage in form of (first) deprecation messages and (then) removal.

Throwing breaks pages rather easily. Note, browsers not supporting any CustomElement stuff, don't throw.

@dglazkov I agree with @smaug---- that throwing breaks badly libraries and sites that already bet on what html5rocks and all documentation around Custom Elements 'till now promoted.

I wouldn't throw and if it's easy for developers to use new second argument, it's even easier for blink to keep what's there already and warn instead.

Regardless, the reason my polyfill supports both is to indeed help developers migrating to the new V1 API so if blink will start throwing, I'll catch that, and switch blink to a polyfilled blink which would be inconvenient for everyone.

WebSQL is still there and it worked well for many, same goes for Custom Elements V0, imo.
𝄞 _Please don't throw_ ♩♫♬

If Chrome ships a breaking change soon we might be able to turn this around. Firefox could most likely follow were that to happen. I'm not going to recommend Firefox to ship a breaking change now though if they want to ship v1 faster than Chrome.

at least we should agree nobody should throw anything unless the new API is fully available and widely deployed, otherwise developers will have broken sites with no way to fix them.

Thank you.

FYI: Firefox nightly is already throwing when passing a string as second argument to document.createElement(el, is) which broke everything online that isn't updated to my latest v1.0.7 polyfill that catches that. Bad fox!

I don't think it's appropriate to castigate browser vendors for following the specification.

I don't think "_castigate_" is an appropriate word for my basic follow up of what happened out there and specs here are changing daily so all I am saying: you are throwing away 2 years of your own Web promotion about this stuff.

If you like this precedent, go on throwing right away: I'm sure devs that believed in V0 and followed Chrome implementing it will love such decision.

/sarcasm

@WebReflection I'd like to remind you of the Code of Ethics and Professional Conduct that governs your participation in this repository. Sarcasm, "bad fox!", and other forms of derogatory and disrespectful interaction with members of the working group are not welcome here. Please consider this a warning.

@domenic I don't think you should bring your (documented) historic personal hostility against me to this group neither, picking on a sentence like "Bad fox" could be, after years of my personal contribution to MDN and Mozilla.

However, I'll note the warning and avoid interactions that brings real-world developers experience I'm aware of in here.

Thank you. It's important to maintain a civil environment, so I'm glad you're taking the warning seriously.

What's left to do on this bug, or can it be closed?

FWIW, Blink is starting to implement deprecation warnings, etc.

@dominiccooney well, we haven't determined yet which way the specification needs to go, right? Whether we can throw for the second argument being a string or not.

Per https://bugzilla.mozilla.org/show_bug.cgi?id=1338889 createElementNS has the same issue...

Hurray.

@dominiccooney I don't see a warning in Blink for document.createElement("test", "test") or document.createElementNS("test", "test", "test"). What is the status on this?

We're working on it. Blink is reticent to add a deprecation warning until there's a clear path to removal, so this is blocked on a few things:

  • Use counter data. That is now available—the use counter has just reached the stable channel. Use is high though.
  • An alternative for developers. We're implementing customized built-in elements, but it has not shipped yet.

An update here would have been good, since I think that effectively means that if Fx wants to ship custom elements, including support for the is attribute, we just have to support a string argument as well due to all the compatibility fallout and Blink moving slowly.

Yeah, so the way we're doing this is the string argument won't create customized built in elements (the new thing in the HTML living standard); to do that, authors will have to use the dictionary. Here's the code.

The upside of this is no other browser should have to handle the second string argument. At worst it might mean not throwing a TypeError but instead ignoring the argument.

The downside of this is that it makes it harder for authors to move code from document.registerElement to customElements.define because you need to migrate element creation code before/at the same time as definition code, which is a bit painful, and that could slow adoption of customized built-in elements. This cost is all borne by Chrome, which may end up having to support document.registerElement longer. Other browsers already decided not to implement document.registerElement so there's nothing new there.

(The way we have it now authors would have to change definition and creation code at the same time, IIRC. But I think this is overly onerous and we should allow the dictionary to create legacy custom elements so that authors can fix all their createElement/NS calls and then go back and port registerElement to define. But foolip had some questions about that and we've punted that decision for now.)

This is the best we could come up with but I'm open to feedback if there's a way to do it better.

So the status quo is that both Chrome and Firefox allow a string argument here still and I don't see any effort toward changing that.

Firefox will simply ignore the second argument if it's a string. I'm going to change the DOM Standard to align with that.

@annevk note that the webidl you've linked from Firefox is really from Servo. The one you're looking for is:

https://searchfox.org/mozilla-central/rev/5536f71c3833018c4f4e2c73f37eae635aab63ff/dom/webidl/Document.webidl#60

Oops, thanks. Same conclusion fortunately.

FWIW, WebKit does not support the second argument at all.

Was this page helpful?
0 / 5 - 0 ratings