ES in chromium proposal: object literal extensions

[Dan Beam]

Hey all,

Often I see (either intentionally or accidentally) object literal extensions (specifically, property methods) being used in our code.

Polymer({

  privateMethod_() { ... method body ... },

});

We haven't formally discussed this (like I'm doing now), but I think we should go ahead allowing these everywhere.  The alternative is this:

Polymer({

  privateMethod_: function() { ... method body ... },

});

which isn't really much better (it's just longer, basically the same).  Note that we also allow the class syntax, which looks similar:

class ClassName() {

  privateMethod_() { ... method body ... }

}

Cross-browser support for object literal extensions seems to be there (except for property spread which doesn't work on iOS10, but that's being discussed separately):
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Object_initializer#Browser_compatibility

I also tested clang-format[1], closure compiler, and eslint: results.  I don't see anything totally show-stopping; I don't think we use computed property names much and shorthand formatting weirdness seems to only happen sometimes.

Thoughts?

-- Dan

[1] There are a couple formatting bugs; I've filed a bug internally.

[Michael Giuffrida]

For method properties, it might be confusing not to allow the shorthand in object literals, given that it's allowed in class definitions.

But computed and shorthand properties are weird, IMO. I'd prefer if literals were explicit about their property names and values. If we're generating property names as in this example:

const o = {
  [prop]: 'hey',
  ['b' + 'ar']: 'there',
};

I would ask why we aren't using a Map object instead.

[Dan Beam]

On Fri, Jan 4, 2019 at 12:35 PM Michael Giuffrida <mich...@chromium.org> wrote:

For method properties, it might be confusing not to allow the shorthand in object literals, given that it's allowed in class definitions.

Right, so I'm interpreting this as a +1 for method properties specifically.

 

But computed and shorthand properties are weird, IMO. I'd prefer if literals were explicit about their property names and values. If we're generating property names as in this example:

const o = {
  [prop]: 'hey',
  ['b' + 'ar']: 'there',
};

I would ask why we aren't using a Map object instead.

I also agree with this (I'm not sure which use cases this syntax addresses, and especially when using this would result in clearer code).

So you'd generally only encourage method properties?  (So we'd split the section in es.md?)

-- Dan

 

On Friday, January 4, 2019 at 12:28:08 PM UTC-8, Dan Beam wrote:

Hey all,

Often I see (either intentionally or accidentally) object literal extensions (specifically, property methods) being used in our code.

Polymer({

  privateMethod_() { ... method body ... },

});

We haven't formally discussed this (like I'm doing now), but I think we should go ahead allowing these everywhere.  The alternative is this:

Polymer({

  privateMethod_: function() { ... method body ... },

});

which isn't really much better (it's just longer, basically the same).  Note that we also allow the class syntax, which looks similar:

class ClassName() {

  privateMethod_() { ... method body ... }

}

Cross-browser support for object literal extensions seems to be there (except for property spread which doesn't work on iOS10, but that's being discussed separately):
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Object_initializer#Browser_compatibility

I also tested clang-format[1], closure compiler, and eslint: results.  I don't see anything totally show-stopping; I don't think we use computed property names much and shorthand formatting weirdness seems to only happen sometimes.

Thoughts?

-- Dan

[1] There are a couple formatting bugs; I've filed a bug internally.

--
--
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/2e668377-8831-4869-8db4-de3579081c84%40chromium.org.

[Michael Giuffrida]

Yes, that's my opinion -- allow method definitions [spec] (whether it's in a class body or object initializer), but either:

• ban computed property names, or for simplicity:
• don't mention computed property names, and leave it to reviewers to judge on a case by case basis

[Hector Carmona]

+1 for method properties, code is cleaner to read and our tooling seems to be happy with it

--

Hector Carmona |

Software Engineer |

hcar...@google.com

[Alan Cutter]

On Saturday, 5 January 2019 07:35:08 UTC+11, Michael Giuffrida wrote:

For method properties, it might be confusing not to allow the shorthand in object literals, given that it's allowed in class definitions.

But computed and shorthand properties are weird, IMO. I'd prefer if literals were explicit about their property names and values. If we're generating property names as in this example:

const o = {
  [prop]: 'hey',
  ['b' + 'ar']: 'there',
};

I would ask why we aren't using a Map object instead.

Computed property names are particularly convenient when combined with Symbols.

E.g.:

class DataStream {
  [Symbol.asyncIterator]() {
    return { next() { ... } }
  }
};
for await (const item of new DataStream(backendUrl)) {
  ...
}

[K Moon]

I was surprised to see these still banned, as I see this syntax a lot externally these days:

https://chromium.googlesource.com/chromium/src/+/master/styleguide/web/es.md#Object-Literal-Extensions

Was there any kind of conclusion to this thread?

[John Lee]

Bumping this thread.

Adding a case for computed property names. They can be also be useful in cases where you need to define a few properties on an object based on some condition.

Before

const myStyles = {
 width:'200px',
 height: '200px',
};
if (isRTL()) {
 myStyles['margin-left'] = '20px';
 myStyles['padding-left'] = '20px';
} else {
 myStyles['padding-right'] = '20px';
 myStyles['margin-right'] = '20px';
}

After

const paddingInlineStart = isRTL() ? 'padding-left' : 'padding-right';
const marginInlineStart = isRTL()? 'margin-left' : 'margin-right';
const myStyles = {
  width: '200px',
  height: '200px',
  [paddingInlineStart]: '20px',
  [marginInlineStart]: '20px',
};

Also seems to have good Closure compiler support:

// Throws an error saying myProperty is not defined.

const myObj = {

  [myProperty]: 1,

};

// Does not throw an error.

const myProperty = 'one';

const myObj = {

  [myProperty]: 1,

};

// Throws an error saying myProperty needs to be string or symbol.

const myProperty = function() {};

const myObj = {

  [myProperty]: 1,

};

[PhistucK]

This example worries me... Why not write style in, well, CSS? Why do you use JavaScript in the first place?

(I have almost no experience with Polymer, is this advocated/encouraged by it? I hope not...)

And we have margin-inline-start and such (or [dir=rtl]), why would you compute those in JavaScript?

Other than that, computed properties are useful, when using constants defined elsewhere instead of hard coding those strings, for example (or when, well, the property name/index is indeed computed).

☆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...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/9de46f5f-db80-497d-9711-0f9016c440a7%40chromium.org.

[dpapad]

On Thursday, December 19, 2019 at 10:10:48 AM UTC-8, PhistucK wrote:

This example worries me... Why not write style in, well, CSS? Why do you use JavaScript in the first place?

It's a workaround for https://bugs.chromium.org/p/chromium/issues/detail?id=1025390, used at https://cs.chromium.org/chromium/src/chrome/browser/resources/tab_strip/tab.js?l=40.

 

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

[Dan Beam]

On Thu, Dec 19, 2019 at 10:09 AM PhistucK <phis...@gmail.com> wrote:

This example worries me... Why not write style in, well, CSS? Why do you use JavaScript in the first place?

(I have almost no experience with Polymer, is this advocated/encouraged by it? I hope not...)

And we have margin-inline-start and such (or [dir=rtl]), why would you compute those in JavaScript?

I agree that CSS :dir(ltr|rtl) / {margin,padding}-inline-{start,end} are probably typically the right answers.  But I also think John's example holds value for a nice way to initialize a literal with dynamic keys (among other static ones) regardless of if done for the purpose of styling.

I think we haven't heard any showstopping objections in 1y (happy birthday to this thread!).

There are some issues with regard to formatting exotic computed property names (I filed an internal clang-format bug that I verified is still an issue and just pinged).

Every other tool seems to work swimmingly for me (closure compiler, uglify, polymer-bundler, eslint, and [mostly] clang-format).  

I'll make a CL to move this to the "Allowed" list.

Concretely: I'd love to shorten a bunch of duplicative return {Blah: Blah, Blee: Blee}; "export" statements in our old cr.define() module system and a bunch of ": function(" from our ES5 classes.  I'll do this soon.

-- Dan

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CABc02_%2B2e%2B1POk%2BTRgHft%2BGY_CYQF07bsGQ_DMiP3zvqKkY4BA%40mail.gmail.com.

[K. Moon]

Thanks for following up on this!

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