Chromium Javascript Style guide: 'class' and custom elements

[Adam Rice]

'class' is listed under "Features To Be Discussed" in the "ES6 Support In Chromium" document, which means it's currently disallowed for Chromium code.

Custom elements v1, ie. the standardised version of custom elements, require the use of the ES6 class syntax. I think this makes the prohibition problematic.

I propose moving classes to the "Allowed Features" section.

[PhistucK]

But can you use custom elements v1? Not all of the supported iOS versions support them (or ECMAScript 2015 classes).

☆PhistucK

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAHixhFoprgS8z3ZE_TKJ%2BRFy%2BzKcfOZw%2BLbFOG%3DetzivpX6teg%40mail.gmail.com.

[Adam Rice]

Good point. Is it worth splitting iOS and non-iOS cases?

[Dan Beam]

On Wed, Jun 14, 2017 at 12:13 AM, Adam Rice <ri...@chromium.org> wrote:

Good point. Is it worth splitting iOS and non-iOS cases?

I think it's probably just worth noting "this doesn't work on iOS" on the feature.  There'll probably be a bunch of things we want to use now that don't work on iOS.  Because there's not a ton of UI in web tech on iOS, we shouldn't let it hold us back.

Additionally, using Chrome's closure compiler pass and the class syntax was throwing until 2 days ago (thanks, tbreisacher@!).  We haven't been able to pull that rev into src/third_party/closure_compiler/compiler/compiler.jar yet because of other breakages.

On 14 June 2017 at 15:44, PhistucK <phis...@gmail.com> wrote:

But can you use custom elements v1? Not all of the supported iOS versions support them (or ECMAScript 2015 classes).

☆PhistucK

On Wed, Jun 14, 2017 at 9:00 AM, Adam Rice <ri...@chromium.org> wrote:

'class' is listed under "Features To Be Discussed" in the "ES6 Support In Chromium" document, which means it's currently disallowed for Chromium code.

Custom elements v1, ie. the standardised version of custom elements, require the use of the ES6 class syntax. I think this makes the prohibition problematic.

Note: you can like kinnnnda use ES5 classes.  Are you trying to use Polymer or just vanilla custom elements?

I propose moving classes to the "Allowed Features" section.

Do you know which JS tools (closure compiler, clang-format, eslint, uglifyjs) work with class?  Does it result in potential performance problems? 

-- Dan

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAHixhFoprgS8z3ZE_TKJ%2BRFy%2BzKcfOZw%2BLbFOG%3DetzivpX6teg%40mail.gmail.com.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAHixhFoERPuvYeeAV%2BkVzTFu8N8O3v03q1ce4JRAnWRdwsN06A%40mail.gmail.com.

[Demetrios Papadopoulos]

FYI, I am working on ensuring that ES6 usage is not blocked by any of our tools (please follow crbug.com/671426).

Latest blocking issue was in Closure compiler where using the --chrome_pass flag along with ES6 classes would result  in an internal compiler error. This has been addressed, and I am in the process of rolling a new Closure Compiler version and verifying that everything works as expected.

Thanks,

Demetrios

[Adam Rice]

On 15 June 2017 at 02:48, Dan Beam <db...@chromium.org> wrote:

Note: you can like kinnnnda use ES5 classes.  Are you trying to use Polymer or just vanilla custom elements?

Actually, I'm not using either within Chromium myself. It just stood out as an obvious issue when I was reading the doc.

Do you know which JS tools (closure compiler, clang-format, eslint, uglifyjs) work with class?  Does it result in potential performance problems? 

I have personally tested closure compiler, clang-format and eslint. Closure works as long as the output language is set to ES5. clang-format and eslint work great.

My knowledge of uglifyjs is second-hand, but last time I heard you needed to use the dev version for ES6 support.

[dpapad]

FYI, I am about to land https://codereview.chromium.org/2925723005, which has an updated version of Closure compiler, which handles --chrome_pass and ES6 classes correctly. I am also moving ES6 classes to the "allowed" ES6 features at https://codereview.chromium.org/2940233002, and plan to land it after Closure compiler has been updated.

[Demetrios Papadopoulos]

@adamk: Should we expect any performance impact by using ES6 classes (compared to their ES5 equivalents)?

[Adam Klein]

Regarding the performance of ES6 classes in V8, construction and usage of class instances should be similar to that of ES5-style classes.

The thing that's lagging a little bit is the speed of class definition (that is, the creation of the class along with its prototype object and methods), either via class declarations or class expressions. ishell (CCed) is working on that problem right now (with a design doc containing lots of details at https://docs.google.com/document/d/18WMEeBWxm2xw7ou09PaMAbhap8CBkBPpzFz8o6PBXP8/edit).

Note that it's somewhat uncommon for class definition to be the bottleneck: in a "normal" sort of program, you define your classes at the beginning, and then use them throughout the runtime of the program. So I wouldn't recommend Chromium should block on ishell's work unless the classes in question are being defined inside hot code.

[Demetrios Papadopoulos]

Thanks for the information regarding performance. So to sum up this discussion, ES6 classes have been moved to the "allowed" section of the JS styleguide and are already being used in a couple of places.

Latest example is at https://codereview.chromium.org/2954863003, where I was able to perform ES5 to ES6 class conversion in a semi-automated way.

Thanks,

Demetrios

[Dominic Cooney]

On Wed, Jun 14, 2017 at 3:00 PM, Adam Rice <ri...@chromium.org> wrote:

'class' is listed under "Features To Be Discussed" in the "ES6 Support In Chromium" document, which means it's currently disallowed for Chromium code.

Custom elements v1, ie. the standardised version of custom elements, require the use of the ES6 class syntax.

This is not the case. There is a requirement that the argument to define is a constructor, but I believe functions are constructors. Here's an example:

http://jsbin.com/lewasidube/edit?html,output

<!DOCTYPE html>

<script>

function MyElement() {

  return Reflect.construct(HTMLElement, [], MyElement);

}

MyElement.prototype = Object.create(HTMLElement.prototype, {

  connectedCallback: {

    value: function() {

      alert('Beware the Ides of March');

    }

  }

});

customElements.define('my-element', MyElement);

</script>

<my-element>Lend me your ears</my-element>

While you do have to call the HTMLElement constructor, you don't need HTMLElement.prototype on the prototype chain as I have it here, although that is a useful convention.

I think this makes the prohibition problematic.

I propose moving classes to the "Allowed Features" section.

--

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAHixhFoprgS8z3ZE_TKJ%2BRFy%2BzKcfOZw%2BLbFOG%3DetzivpX6teg%40mail.gmail.com.

[PhistucK]

​A better example would not ​use the probably also unsupported Reflect. :(

☆PhistucK

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAHnmYQ_XRyqnbi%3DKhSEJ%3DN9S4ea_babp%2Bju02%3DTgzETFu3cMSQ%40mail.gmail.com.

[Dominic Cooney]

I think you have to either use classes, or use Reflect.construct, because those are the two things which can set new.target and you have to do that.

I think Reflect.construct is supported everywhere customElements.define is at the moment, so I think it is OK to use it.

[PhistucK]

I think classes are also supported everywhere customElements.define is at the moment as well?

☆PhistucK

[Dominic Cooney]

Yes, that is my understanding.

[Dan Beam]

On Thu, Jun 29, 2017 at 12:42 AM, Dominic Cooney <domi...@chromium.org> wrote:

Yes, that is my understanding.

On Jun 29, 2017 3:05 PM, "PhistucK" <phis...@gmail.com> wrote:

I think classes are also supported everywhere customElements.define is at the moment as well?

class is a new syntax and trips up many older source-reading tools.  customElements.define() looks like a good old method call (no new syntax).

This is why chromium has been slower to adopt ES6 features that introduce new syntaxes: we lint and check our code in many [usually] helpful ways.

-- Dan

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAHnmYQ9pVECLrYPV3tQxxm2ppogm%3DP5mVV2PJpNqn93w%3D_NDCw%40mail.gmail.com.