Using new JS language features in our code

86 views
Skip to first unread message

Raymes Khoury

unread,
Sep 28, 2015, 12:09:24 AM9/28/15
to Chromium-dev, Sam McNally
A few times now, sammc@ and I have had a friendly disagreement about using new JS language features in our codebase.  These are features like:
-arrow functions
-async functions (using generators)
-and other ES6 features

Sam is more aggressive in wanting to use new features as they might enable more readable or more concise code. I’m more defensive, being wary of new features making the code less readable, especially if a feature is not broadly understood or is used inappropriately.

I think the tension is probably healthy but it does seem like it would benefit from broader discussion and documenting the result of those discussions (in a similar way to C++11 features). I’m all for using new language features if there is consensus on the team. Note that currently the JS style guide makes no mention of these features.

Does anyone have thoughts about the appropriate process here?

Thanks!
Raymes

Marshall Greenblatt

unread,
Sep 28, 2015, 4:54:57 AM9/28/15
to Raymes Khoury, Chromium-dev, Sam McNally
I think we also need to consider how long the new JS features have been supported by Chrome and whether they'll be exposed to older browser versions. For example, the DevTools front-end (which is exposed via remote debugging) should probably avoid using features that are not yet supported by the current Chrome stable channel.
 

Thanks!
Raymes

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

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

Jakob Kummerow

unread,
Sep 28, 2015, 7:14:28 AM9/28/15
to magree...@gmail.com, Raymes Khoury, Chromium-dev, Sam McNally
In general, it also makes sense to wait at least a milestone or two after support for a given language feature has first been shipped to make sure it's actually stable enough to stick around. We've had to un-ship features before due to critical issues being discovered, it might happen again, and it would sure be a shame if we couldn't do that because other parts of Chrome already rely on it.

Also, when it comes to ES 2015 features, V8 currently prioritizes supporting them at all over making them fast. As an example, for-of loops are still quite a bit slower than classic "for(var i = 0; i < a.length; i++)" loops. So if you care about speed, please measure carefully if the implementation based on the fancy new feature is fast enough for your needs.
(Obviously our long-term goal is to provide awesome speed especially for useful new language features, but that'll take time.)

Charlie Andrews

unread,
Sep 28, 2015, 8:02:01 AM9/28/15
to jkum...@chromium.org, magree...@gmail.com, Raymes Khoury, Chromium-dev, Sam McNally, Nat Duca
FWIW, we've been having a lot of these same discussions within the speed infra and perf insights teams too.

The tack we're taking is:
  1. Wait until the feature is stable
  2. Write tests to make sure that it doesn't have a large negative impact on performance
  3. Because other teams depend on us, add the feature in an easily-revertable patch, wait a while to see if anyone breaks, then green light the feature for general use
I definitely agree with Raymes that because Chrome's so widely used, a healthy dose of caution is good. As far as:

I’m more defensive, being wary of new features making the code less readable, especially if a feature is not broadly understood or is used inappropriately.

My opinion is that the web is a constantly changing platform, and it's our responsibility as developers to keep up. Lots of useful things are initially difficult to use (pointers) or hard to read for the uninitiated (arrow functions), but that doesn't mean those features should be avoided. IMO the reason for caution lies more with stability and performance issues.

Charlie Andrews | Software Engineer | char...@google.com
 

PhistucK

unread,
Sep 28, 2015, 9:41:17 AM9/28/15
to char...@google.com, Jakob Kummerow, magree...@gmail.com, Raymes Khoury, Chromium-dev, Sam McNally, Nat Duca
One more (and stronger) reason I remember reading recently (or not so recently) was the Chrome for iOS might also be running this JavaScript (some Web UI, for example) and since it is not V8 based and since Chrome still supports iOS 7 (which has next to nothing - 12% - ECMAScript 2015 features), it will fail there on script startup (since this is a syntax error).

So, unless you want to make that distinction (I remember people arguing against that, for example, for Chrome OS code in Chromium using C++11 library features) between iOS shared code and non iOS code, ECMAScript 2015 features should probably not be used yet.


PhistucK

Stuart Morgan

unread,
Sep 28, 2015, 10:05:01 AM9/28/15
to phis...@gmail.com, char...@google.com, Jakob Kummerow, magree...@gmail.com, Raymes Khoury, Chromium-dev, Sam McNally, Nat Duca
The current plan of record actually is to make that distinction, so that JS that needs to support iOS is specifically separated out into a subdirectory, and only that JS needs to be restricted to being iOS-friendly. Course version of the split will be done during componentization of features using WebUI, and then can be refined over time.

-Stuart

Raymes Khoury

unread,
Sep 28, 2015, 8:23:24 PM9/28/15
to Stuart Morgan, phis...@gmail.com, char...@google.com, Jakob Kummerow, magree...@gmail.com, Chromium-dev, Sam McNally, Nat Duca, Nico Weber
Should we have a process similar to C++11 features? That is:
> You can propose to make a feature available or to ban a feature by sending an
> email to chromium-dev. Ideally include a short blurb on what the feature is,
> and why you think it should or should not be allowed. Ideally, the list will
> arrive at some consensus and the wiki page will be updated to mention that
> consensus. If there's no consensus, src/styleguide/C++/OWNERS get to decide --
> for divisive features, we expect the decision to be to not use the feature yet
> and possibly discuss it again a few months later, when we have more experience
> with the language.

I think many of the concerns raised here would feed into decisions:
-Readability
-Availability throughout the codebase (ability to use the feature consistently)
-Stability
-Performance
-Other benefits the feature brings

Would this process be too heavyweight for JS usage in chromium? Any other suggestions/concerns?

Thanks,
Raymes

Dan Beam

unread,
Oct 8, 2015, 11:11:23 PM10/8/15
to Raymes Khoury, Stuart Morgan, PhistucK Productions, char...@google.com, Jakob Kummerow, magree...@gmail.com, Chromium-dev, Sam McNally, Nat Duca, Nico Weber
On Mon, Sep 28, 2015 at 5:21 PM, Raymes Khoury <ray...@chromium.org> wrote:
Should we have a process similar to C++11 features? That is:
> You can propose to make a feature available or to ban a feature by sending an
> email to chromium-dev. Ideally include a short blurb on what the feature is,
> and why you think it should or should not be allowed. Ideally, the list will
> arrive at some consensus and the wiki page will be updated to mention that
> consensus. If there's no consensus, src/styleguide/C++/OWNERS get to decide --
> for divisive features, we expect the decision to be to not use the feature yet
> and possibly discuss it again a few months later, when we have more experience
> with the language.

I think many of the concerns raised here would feed into decisions:
-Readability
-Availability throughout the codebase (ability to use the feature consistently)
-Stability
-Performance
-Other benefits the feature brings

Would this process be too heavyweight for JS usage in chromium? Any other suggestions/concerns?

Concern: some web UI code runs on iOS (which doesn't use v8/blink).

For instance: chrome://history on iOS throws when trying to call (the undefined) document.registerElement().  It seems to be harmless (?) as it's been broken for a while :(.

We hit the same issues with `let` and `Map` and ....

-- Dan
Reply all
Reply to author
Forward
0 new messages