RESOLVED FIXED 175815
REGRESSION(r216718): Speedometer 2.0: Fix vanilla JS examples
https://bugs.webkit.org/show_bug.cgi?id=175815
Summary REGRESSION(r216718): Speedometer 2.0: Fix vanilla JS examples
Ryosuke Niwa
Reported 2017-08-22 00:50:53 PDT
Vanilla JS & ES2015 test cases got broken again in https://trac.webkit.org/changeset/217107 We can't use new Date().getTime() to generate an unique ID.
Attachments
Fixes the bug (256.47 KB, patch)
2017-08-22 00:56 PDT, Ryosuke Niwa
saam: review+
Ryosuke Niwa
Comment 1 2017-08-22 00:56:49 PDT
Created attachment 318744 [details] Fixes the bug
Ryosuke Niwa
Comment 2 2017-08-22 00:57:52 PDT
Comment on attachment 318744 [details] Fixes the bug bundle.app* files are generated from npm run build. See changes in PerformanceTests/Speedometer/resources/todomvc/vanilla-examples/es2015-babel-webpack/src/store.js PerformanceTests/Speedometer/resources/todomvc/vanilla-examples/es2015/src/store.js for the actual code/test change.
Mathias Bynens
Comment 3 2017-08-22 01:11:14 PDT
Comment on attachment 318744 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=318744&action=review > PerformanceTests/Speedometer/resources/todomvc/vanilla-examples/es2015-babel-webpack/src/store.js:5 > +var uniqueID = 0; `vanilla-examples/es2015/src/store.js` has the same line but uses `let` instead of `var` and starts at `1` instead of `0`. Is there a reason for these subtle differences? If so, we may want to add a comment saying so.
Mathias Bynens
Comment 4 2017-08-22 01:40:21 PDT
How did https://trac.webkit.org/changeset/220043/webkit(In reply to Ryosuke Niwa from comment #0) > Vanilla JS & ES2015 test cases got broken again in > https://trac.webkit.org/changeset/217107 That doesn’t seem like the right link. This bug’s title refers to R220043 but that patch didn’t touch any JavaScript: https://trac.webkit.org/changeset/220043 Did you mean R216718 instead? https://trac.webkit.org/changeset/216718
Ryosuke Niwa
Comment 5 2017-08-22 13:54:46 PDT
(In reply to Mathias Bynens from comment #3) > Comment on attachment 318744 [details] > Fixes the bug > > View in context: > https://bugs.webkit.org/attachment.cgi?id=318744&action=review > > > PerformanceTests/Speedometer/resources/todomvc/vanilla-examples/es2015-babel-webpack/src/store.js:5 > > +var uniqueID = 0; > > `vanilla-examples/es2015/src/store.js` has the same line but uses `let` > instead of `var` and starts at `1` instead of `0`. Is there a reason for > these subtle differences? If so, we may want to add a comment saying so. It looked like all variable declarations in this file was using `var` so I used that to be consistent. (In reply to Mathias Bynens from comment #4) > How did https://trac.webkit.org/changeset/220043/webkit(In reply to Ryosuke > Niwa from comment #0) > > Vanilla JS & ES2015 test cases got broken again in > > https://trac.webkit.org/changeset/217107 > > That doesn’t seem like the right link. > > This bug’s title refers to R220043 but that patch didn’t touch any > JavaScript: https://trac.webkit.org/changeset/220043 > > Did you mean R216718 instead? https://trac.webkit.org/changeset/216718 Oops, fixed.
Saam Barati
Comment 6 2017-08-22 16:04:08 PDT
Comment on attachment 318744 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=318744&action=review >>> PerformanceTests/Speedometer/resources/todomvc/vanilla-examples/es2015-babel-webpack/src/store.js:5 >>> +var uniqueID = 0; >> >> `vanilla-examples/es2015/src/store.js` has the same line but uses `let` instead of `var` and starts at `1` instead of `0`. Is there a reason for these subtle differences? If so, we may want to add a comment saying so. > > It looked like all variable declarations in this file was using `var` so I used that to be consistent. > > (In reply to Mathias Bynens from comment #4) As Mathias asked, is there a reason to use 0 instead of 1 or vice versa?
Ryosuke Niwa
Comment 7 2017-08-22 16:06:16 PDT
Oh oops, clearly, I need to start the value at 1.
Ryosuke Niwa
Comment 8 2017-08-22 16:11:07 PDT
Radar WebKit Bug Importer
Comment 9 2017-08-22 16:11:42 PDT
Note You need to log in before you can comment on or make changes to this bug.