Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2509183002:
Initial pass creating the ES6 feature doc (Closed)
DescriptionInitial pass creating the ES6 feature doc
BUG=671426
NOTRY=true # blame dbeam@ if something bad happens
Committed: https://crrev.com/84ef86caae2a84f0dd898972ef721d97c02ac6e3
Cr-Commit-Position: refs/heads/master@{#436479}
Patch Set 1 #
Total comments: 14
Patch Set 2 : update example styles to match styleguide; move Promise to allowed #Patch Set 3 : comply to 80-col #Patch Set 4 : convert page to accordion on page load #Patch Set 5 : clarify arrow function explanation #
Total comments: 77
Messages
Total messages: 27 (11 generated)
scottchen@chromium.org changed reviewers: + dbeam@chromium.org, dpapad@chromium.org
https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md File docs/es6-chromium.md (right): https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md#newcod... docs/es6-chromium.md:109: for(let i = 0; i < 3; i++) { s/let/var? https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md#newcod... docs/es6-chromium.md:120: } I think there is more to the "let" vs "var" distinction that is not depicted by the above example. var bar = 1; var bar = 1; // No error thrown. let foo = 1; let foo = 1'; // TypeError: Identifier 'foo' has already been declared https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md#newcod... docs/es6-chromium.md:138: const gravity = 9.81; Add an example showing the error shown when trying to assign a new value to a const. I think this is helpful advocating the benefits of 'const'. 'TypeError: Assignment to constant variable.' https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md#newcod... docs/es6-chromium.md:185: return new Rectangle("default", 0, 0, 100, 100) Indent is off by 2. Should double quotes be switched to single quotes in all examples, given that we prefer single quotes in JS files? https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md#newcod... docs/es6-chromium.md:200: ## `Promise` We are already using this extensively, so perhaps is this the one we can already move to the "allowed" section? https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md#newcod... docs/es6-chromium.md:341: var message = `${ greeting }, Can we remove the extra spaces around the brackets? ${greeting}
https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md File docs/es6-chromium.md (right): https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md#newcode16 docs/es6-chromium.md:16: <script src="https://code.jquery.com/jquery-2.2.4.min.js"></script> why are we using jquery?
scottchen@google.com changed reviewers: + scottchen@google.com
https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md File docs/es6-chromium.md (right): https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md#newcode16 docs/es6-chromium.md:16: <script src="https://code.jquery.com/jquery-2.2.4.min.js"></script> On 2016/11/22 02:37:11, Dan Beam wrote: > why are we using jquery? I had it to save me some time to implement a fixed top bar that tells you which section you're looking at. According to our offline discussion we came to the conclusion of improving readability with other methods, reiterating here for visibility. https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md#newcod... docs/es6-chromium.md:109: for(let i = 0; i < 3; i++) { On 2016/11/21 22:59:55, dpapad wrote: > s/let/var? Done. Copy paste error. https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md#newcod... docs/es6-chromium.md:120: } On 2016/11/21 22:59:55, dpapad wrote: > I think there is more to the "let" vs "var" distinction that is not depicted by > the above example. > > var bar = 1; > var bar = 1; // No error thrown. > > let foo = 1; > let foo = 1'; // TypeError: Identifier 'foo' has already been declared Done. https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md#newcod... docs/es6-chromium.md:138: const gravity = 9.81; On 2016/11/21 22:59:55, dpapad wrote: > Add an example showing the error shown when trying to assign a new value to a > const. I think this is helpful advocating the benefits of 'const'. > > 'TypeError: Assignment to constant variable.' Done. https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md#newcod... docs/es6-chromium.md:185: return new Rectangle("default", 0, 0, 100, 100) On 2016/11/21 22:59:55, dpapad wrote: > Indent is off by 2. > Should double quotes be switched to single quotes in all examples, given that we > prefer single quotes in JS files? Done. Yeah I think it's a good idea to change it to our style. https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md#newcod... docs/es6-chromium.md:200: ## `Promise` On 2016/11/21 22:59:55, dpapad wrote: > We are already using this extensively, so perhaps is this the one we can already > move to the "allowed" section? Done. https://codereview.chromium.org/2509183002/diff/1/docs/es6-chromium.md#newcod... docs/es6-chromium.md:341: var message = `${ greeting }, On 2016/11/21 22:59:55, dpapad wrote: > Can we remove the extra spaces around the brackets? > ${greeting} Done.
scottchen@chromium.org changed reviewers: - scottchen@google.com
https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md File docs/es6-chromium.md (right): https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:264: class Rectangle extends Shape { Can you also include the definition of the Shape class? It will make the example more complete. Also, it seems that the move() method belongs in the superclass, since it only accesses properties of the superclass. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:340: // for f(1, 2, 3, 4, 5)... These comments threw me off. It looks like a commented out "for" loop that is missing open/closing brackets. The '...' and the indentation are the culprits I think. How about the following? function f(x, y, ...a) { console.log(x, y, a); } f(1, 2, 3, 4, 5) // prints "1 2 [3, 4, 5]" https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:364: f(1, 2, ...params) === 9; After reading this line, I am still not sure how does |params| gets represented inside the function Perhaps it would help if you define that function here and add console.logs() inside it to illustrate. Also comparing the result to "9" seems arbitrary, and could not tell how it benefits the example. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:390: var obj = {x, y}; //equivalent to {x:x, y:y} Where is y defined? I understand that this is not complete code, but let's make it as less pseudo as possible. Pasting lines 386-390 to a dev console throws an error. It would be more useful if I can paste it and see |obj| successfully defined. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:770: Let's boost this example to be a bit more descriptive. var map = new Map(); map.size === 0; // true map.get('foo'); // undefined var key = {}; map.set(key, 123); map.size === 1; // true map.has(key); // true map.get(key); // 123 map.delete(key); map.has(key); // false; map.size === 0; // true https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:788: var obj = {}; obj is not used in this example. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:792: set.add(123); set.size == 1; // true
https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md File docs/es6-chromium.md (right): https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:60: document.addEventListener("DOMContentLoaded", function(event) { prefer single quotes always https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:62: document.querySelectorAll('h2[id]').forEach(function(header){ document.querySelectorAll('h2[id]').forEach(function(header){ space here == ^^ https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:68: var ele = header; ele -> el everywhere https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:69: while(ele && ele.tagName !== 'HR') { while(ele ^^ space here https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:76: header.addEventListener('click', () => { you should arguably wait until we support => before doing this ;) https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:116: relieving developers from "callback hell". can we steal more from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje... ? The Promise object is used for asynchronous computations. A Promise represents a value which may be available now, or in the future, or never. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:135: }); can you use less ES6 features in this example? how about: /** @type {!Promise} */ var fullyLoaded = new Promise(function(resolve) { function isLoaded() { return document.readyState == 'complete'; } if (isLoaded()) { resolve(); } else { document.onreadystatechange = function() { if (isLoaded()) resolve(); }; } }); ... loaded.then(startTheApp); https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:158: Declare variable that exists within the block scope. `let` can generally be `let` declares a variable within the scope of a block. This differs from `var`, which uses function level scope. then we should borrow the example from MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/... function varTest() { var x = 1; if (true) { var x = 2; // same variable! console.log(x); // 2 } console.log(x); // 2 } function letTest() { let x = 1; if (true) { let x = 2; // different variable console.log(x); // 2 } console.log(x); // 1 } https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:201: is now a block scope declaration. i would drop this an just say "Also note that in Chrome, `const` is block scoped like `let`" as it doesn't matter what IE does in our case https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:209: gravity === 9.81; // true can we put an example of an object? const frobber = {isFrobbing: true}; frobber = {isFrobbing: false}; // TypeError: Assignment to constant variable. frobber.isFrobbing = false; // works See also: [Object.freeze()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/freeze). https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:226: especially over `goog.bind(f, this)`. we don't use closure, so goog.bind() isn't really as popular in our codebase... https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:246: () => {return 1;} // returns 1 nit: two spaces before the comment starts (right now you have 1) https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:263: ``` js class Shape { constructor(id, x, y) { this.x = x; this.y = y; } } https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:270: static defaultRectangle() { more nerd points for static goldenRectangle() { var PHI = (1 + Math.sqrt(5)) / 2; return new Rectangle('golden', 0, 0, PHI, 1); } https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:277: }; can you drop the ; from this? it's only kinda desired when the result of an expression, i.e. let Shape = class { ... }; https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:319: function f(x, y = 7, z = 42) { nit: I think this is a more useful example /** * @param {!Element} element An element to hide. * @param {boolean=} animate Whether to animatedly hide |element|. */ function hide(element, animate = true) { function setHidden() { element.hidden = true; } if (animate) element.animate({...}).then(setHidden); else setHidden(); } https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:340: // for f(1, 2, 3, 4, 5)... On 2016/11/23 01:39:38, dpapad wrote: > These comments threw me off. It looks like a commented out "for" loop that is > missing open/closing brackets. The '...' and the indentation are the culprits I > think. How about the following? > > function f(x, y, ...a) { console.log(x, y, a); } > f(1, 2, 3, 4, 5) // prints "1 2 [3, 4, 5]" i agree but I think this would be even simpler: function usesRestParams(a, b, ...theRest) { console.log(a); // 'a' console.log(b); // 'b' console.log(theRest); // [1, 2, 3] } usesRestParams('a', 'b', 1, 2, 3); https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:364: f(1, 2, ...params) === 9; On 2016/11/23 01:39:38, dpapad wrote: > After reading this line, I am still not sure how does |params| gets represented > inside the function Perhaps it would help if you define that function here and > add console.logs() inside it to illustrate. Also comparing the result to "9" > seems arbitrary, and could not tell how it benefits the example. agreed, can we drop this line until we find a less cryptic example? https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:387: var obj = {[x]: 1}; var prop = 'foo'; var o = { [prop]: 'hey', ['b' + 'ar']: 'there', }; console.log(o); // {foo: 'hey', bar: 'there'} from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/O... https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:390: var obj = {x, y}; //equivalent to {x:x, y:y} var foo = 1; var bar = 2; var o = {foo, bar}; console.log(o); // {foo: 1, bar: 2} https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:396: } var clearSky = { // basically the same as clouds: function() { return 0; } clouds() { return 0; }, color() { return 'blue'; }, }; console.log(clearSky.color()); // 'blue' console.log(clearSky.clouds()); // 0 https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:422: function foo (strings, ...values) { function foo ( drop space ^ https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:431: var newString = foo`bar${42}baz`; // 'bazbar42' can you just drop the last 2 examples (and just keep the hello my name is example) https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:490: efficient parsing of arbitrary long input strings, even with an arbitrary arbitrarily long input strings? https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:499: result === 'yy' && re.lastIndex === 5; // true i don't understand this example https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:517: var list = [ 1, 2, 3 ]; note: our ES5 style guide requires no spaces around [array] or {object} literals. please update every [ ... ] to [...] and { ... } to {...} throughout this guide https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:518: var [ a, , b ] = list; var [a, , b] = [1, 2, 3]; // a = 1, b = 3 https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:522: var {width, height, area: a} = rect; define these variables https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:538: h({ name: 'bar', val: 42 }); put the definition and the call of these functions closer together function f(...) { } f() function g(...) { } g(...) https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:603: Convenient operator to iterate over all values of an iterable object. does this include own keys? as in: can we explain how this is this different from for ( in )? https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:609: for (let n of fibonacci) { nit: can you use var instead of let in this example? https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:770: On 2016/11/23 01:39:38, dpapad wrote: > Let's boost this example to be a bit more descriptive. > > var map = new Map(); > map.size === 0; // true > map.get('foo'); // undefined > > var key = {}; > map.set(key, 123); > map.size === 1; // true > map.has(key); // true > map.get(key); // 123 > > map.delete(key); > map.has(key); // false; > map.size === 0; // true agreed https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:792: set.add(123); On 2016/11/23 01:39:38, dpapad wrote: > set.size == 1; // true agreed https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:860: new UInt8ClampedArray() nit: new UInt8ClampedArray(); https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:885: }); var keyTracker = new Proxy({}, { keysCreated: 0, get (receiver, key) { if (key in receiver) { console.log('key already exists'); } else { ++this.keysCreated; console.log(this.keysCreated + ' keys created!'); receiver[key] = true; } }, }); keyTracker.key1; // 1 keys created! keyTracker.key1; // key already exists keyTracker.key2; // 2 keys created!
Description was changed from ========== initial pass creating the es6 feature doc BUG= ========== to ========== Initial pass creating the es6 feature doc BUG=671426 ==========
in light of the recent deprecation proposal of shadow dom v0 (and shadow dom v1 requiring es6 classes), I'd like to land this and follow up with the issues I mentioned
Description was changed from ========== Initial pass creating the es6 feature doc BUG=671426 ========== to ========== Initial pass creating the es6 feature doc BUG=671426 NOTRY=true # blame dbeam@ if something bad happens ==========
Description was changed from ========== Initial pass creating the es6 feature doc BUG=671426 NOTRY=true # blame dbeam@ if something bad happens ========== to ========== Initial pass creating the ES6 feature doc BUG=671426 NOTRY=true # blame dbeam@ if something bad happens ==========
lgtm
The CQ bit was checked by dbeam@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1480986610480900,
"parent_rev": "d1b70172d3cd48dce79318161a823262eab184dc", "commit_rev":
"44cd568070b95fd0e9872ec5e7e2e9477c0caa07"}
Message was sent while issue was closed.
Description was changed from ========== Initial pass creating the ES6 feature doc BUG=671426 NOTRY=true # blame dbeam@ if something bad happens ========== to ========== Initial pass creating the ES6 feature doc BUG=671426 NOTRY=true # blame dbeam@ if something bad happens ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Initial pass creating the ES6 feature doc BUG=671426 NOTRY=true # blame dbeam@ if something bad happens ========== to ========== Initial pass creating the ES6 feature doc BUG=671426 NOTRY=true # blame dbeam@ if something bad happens Committed: https://crrev.com/84ef86caae2a84f0dd898972ef721d97c02ac6e3 Cr-Commit-Position: refs/heads/master@{#436479} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/84ef86caae2a84f0dd898972ef721d97c02ac6e3 Cr-Commit-Position: refs/heads/master@{#436479}
Message was sent while issue was closed.
https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md File docs/es6-chromium.md (right): https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:60: document.addEventListener("DOMContentLoaded", function(event) { On 2016/11/30 23:08:54, Dan Beam wrote: > prefer single quotes always Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:62: document.querySelectorAll('h2[id]').forEach(function(header){ On 2016/11/30 23:08:53, Dan Beam wrote: > document.querySelectorAll('h2[id]').forEach(function(header){ > space here == ^^ Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:68: var ele = header; On 2016/11/30 23:08:53, Dan Beam wrote: > ele -> el everywhere Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:69: while(ele && ele.tagName !== 'HR') { On 2016/11/30 23:08:53, Dan Beam wrote: > while(ele > ^^ > space here Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:76: header.addEventListener('click', () => { On 2016/11/30 23:08:53, Dan Beam wrote: > you should arguably wait until we support => before doing this ;) Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:116: relieving developers from "callback hell". On 2016/11/30 23:08:53, Dan Beam wrote: > can we steal more from > https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje... > ? > > The Promise object is used for asynchronous computations. A Promise represents a > value which may be available now, or in the future, or never. Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:135: }); On 2016/11/30 23:08:54, Dan Beam wrote: > can you use less ES6 features in this example? > > how about: > > /** @type {!Promise} */ > var fullyLoaded = new Promise(function(resolve) { > function isLoaded() { return document.readyState == 'complete'; } > > if (isLoaded()) { > resolve(); > } else { > document.onreadystatechange = function() { > if (isLoaded()) resolve(); > }; > } > }); > > ... > > loaded.then(startTheApp); Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:158: Declare variable that exists within the block scope. `let` can generally be On 2016/11/30 23:08:52, Dan Beam wrote: > `let` declares a variable within the scope of a block. This differs from `var`, > which uses function level scope. > > then we should borrow the example from MDN: > > https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/... > > function varTest() { > var x = 1; > if (true) { > var x = 2; // same variable! > console.log(x); // 2 > } > console.log(x); // 2 > } > > function letTest() { > let x = 1; > if (true) { > let x = 2; // different variable > console.log(x); // 2 > } > console.log(x); // 1 > } Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:201: is now a block scope declaration. On 2016/11/30 23:08:53, Dan Beam wrote: > i would drop this an just say "Also note that in Chrome, `const` is block scoped > like `let`" as it doesn't matter what IE does in our case Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:209: gravity === 9.81; // true On 2016/11/30 23:08:53, Dan Beam wrote: > can we put an example of an object? > > const frobber = {isFrobbing: true}; > frobber = {isFrobbing: false}; // TypeError: Assignment to constant variable. > frobber.isFrobbing = false; // works > > See also: > [Object.freeze()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/freeze). Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:226: especially over `goog.bind(f, this)`. On 2016/11/30 23:08:53, Dan Beam wrote: > we don't use closure, so goog.bind() isn't really as popular in our codebase... Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:246: () => {return 1;} // returns 1 On 2016/11/30 23:08:52, Dan Beam wrote: > nit: two spaces before the comment starts (right now you have 1) Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:263: ``` js On 2016/11/30 23:08:52, Dan Beam wrote: > class Shape { > constructor(id, x, y) { > this.x = x; > this.y = y; > } > } Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:264: class Rectangle extends Shape { On 2016/11/23 01:39:38, dpapad wrote: > Can you also include the definition of the Shape class? It will make the example > more complete. Also, it seems that the move() method belongs in the superclass, > since it only accesses properties of the superclass. Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:270: static defaultRectangle() { On 2016/11/30 23:08:53, Dan Beam wrote: > more nerd points for > > static goldenRectangle() { > var PHI = (1 + Math.sqrt(5)) / 2; > return new Rectangle('golden', 0, 0, PHI, 1); > } Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:277: }; On 2016/11/30 23:08:53, Dan Beam wrote: > can you drop the ; from this? it's only kinda desired when the result of an > expression, i.e. > > let Shape = class { > ... > }; Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:319: function f(x, y = 7, z = 42) { On 2016/11/30 23:08:54, Dan Beam wrote: > nit: I think this is a more useful example > > /** > * @param {!Element} element An element to hide. > * @param {boolean=} animate Whether to animatedly hide |element|. > */ > function hide(element, animate = true) { > function setHidden() { element.hidden = true; } > > if (animate) > element.animate({...}).then(setHidden); > else > setHidden(); > } Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:364: f(1, 2, ...params) === 9; On 2016/11/30 23:08:53, Dan Beam wrote: > On 2016/11/23 01:39:38, dpapad wrote: > > After reading this line, I am still not sure how does |params| gets > represented > > inside the function Perhaps it would help if you define that function here and > > add console.logs() inside it to illustrate. Also comparing the result to "9" > > seems arbitrary, and could not tell how it benefits the example. > > agreed, can we drop this line until we find a less cryptic example? Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:387: var obj = {[x]: 1}; On 2016/11/30 23:08:53, Dan Beam wrote: > var prop = 'foo'; > var o = { > [prop]: 'hey', > ['b' + 'ar']: 'there', > }; > console.log(o); // {foo: 'hey', bar: 'there'} > > from > https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/O... Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:390: var obj = {x, y}; //equivalent to {x:x, y:y} On 2016/11/30 23:08:54, Dan Beam wrote: > var foo = 1; > var bar = 2; > var o = {foo, bar}; > console.log(o); // {foo: 1, bar: 2} Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:396: } On 2016/11/30 23:08:54, Dan Beam wrote: > var clearSky = { > // basically the same as clouds: function() { return 0; } > clouds() { return 0; }, > color() { return 'blue'; }, > }; > console.log(clearSky.color()); // 'blue' > console.log(clearSky.clouds()); // 0 Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:422: function foo (strings, ...values) { On 2016/11/30 23:08:54, Dan Beam wrote: > function foo ( > drop space ^ Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:431: var newString = foo`bar${42}baz`; // 'bazbar42' On 2016/11/30 23:08:53, Dan Beam wrote: > can you just drop the last 2 examples (and just keep the hello my name is > example) Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:490: efficient parsing of arbitrary long input strings, even with an arbitrary On 2016/11/30 23:08:53, Dan Beam wrote: > arbitrarily long input strings? Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:517: var list = [ 1, 2, 3 ]; On 2016/11/30 23:08:53, Dan Beam wrote: > note: our ES5 style guide requires no spaces around [array] or {object} > literals. please update every [ ... ] to [...] and { ... } to {...} throughout > this guide Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:518: var [ a, , b ] = list; On 2016/11/30 23:08:52, Dan Beam wrote: > var [a, , b] = [1, 2, 3]; // a = 1, b = 3 Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:522: var {width, height, area: a} = rect; On 2016/11/30 23:08:54, Dan Beam wrote: > define these variables Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:538: h({ name: 'bar', val: 42 }); On 2016/11/30 23:08:53, Dan Beam wrote: > put the definition and the call of these functions closer together > > function f(...) { > } > f() > > function g(...) { > } > g(...) Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:603: Convenient operator to iterate over all values of an iterable object. On 2016/11/30 23:08:53, Dan Beam wrote: > does this include own keys? as in: can we explain how this is this different > from for ( in )? Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:609: for (let n of fibonacci) { On 2016/11/30 23:08:53, Dan Beam wrote: > nit: can you use var instead of let in this example? Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:770: On 2016/11/30 23:08:53, Dan Beam wrote: > On 2016/11/23 01:39:38, dpapad wrote: > > Let's boost this example to be a bit more descriptive. > > > > var map = new Map(); > > map.size === 0; // true > > map.get('foo'); // undefined > > > > var key = {}; > > map.set(key, 123); > > map.size === 1; // true > > map.has(key); // true > > map.get(key); // 123 > > > > map.delete(key); > > map.has(key); // false; > > map.size === 0; // true > > agreed Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:788: var obj = {}; On 2016/11/23 01:39:38, dpapad wrote: > obj is not used in this example. Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:792: set.add(123); On 2016/11/30 23:08:53, Dan Beam wrote: > On 2016/11/23 01:39:38, dpapad wrote: > > set.size == 1; // true > > agreed Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:860: new UInt8ClampedArray() On 2016/11/30 23:08:53, Dan Beam wrote: > nit: new UInt8ClampedArray(); Done. https://codereview.chromium.org/2509183002/diff/80001/docs/es6-chromium.md#ne... docs/es6-chromium.md:885: }); On 2016/11/30 23:08:53, Dan Beam wrote: > var keyTracker = new Proxy({}, { > keysCreated: 0, > > get (receiver, key) { > if (key in receiver) { > console.log('key already exists'); > } else { > ++this.keysCreated; > console.log(this.keysCreated + ' keys created!'); > receiver[key] = true; > } > }, > }); > > keyTracker.key1; // 1 keys created! > keyTracker.key1; // key already exists > keyTracker.key2; // 2 keys created! Done.
Message was sent while issue was closed.
thakis@chromium.org changed reviewers: + thakis@chromium.org
Message was sent while issue was closed.
vapier asked on chromium-dev why this doc isn't somewhere below styleguide/ That seems like a good question to me, did y'all consider putting it there?
Message was sent while issue was closed.
On 2017/07/21 20:30:22, Nico wrote: > vapier asked on chromium-dev why this doc isn't somewhere below styleguide/ That > seems like a good question to me, did y'all consider putting it there? at the time this was created there was no web style guide. it would've been weird to build upon a (non-existent) ES5 style guide for ES6 (like C++11 doc without the C++ style guide). this was a little less fleshed out than it is now (and now tooling support is better). tl;dr - I think it'd be fine to figure out how to integrate this with styleguide/ (either by moving the file there or pushing into web.md or something). ultimately, it's probably more up to dpapad@ than me, now. |
