Closed
Bug 1217082
Opened 9 years ago
Closed 9 years ago
Remove use of for-each from toolkit/.
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 44
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: arai, Assigned: arai)
References
Details
Attachments
(2 files)
Need to replace non-standard for-each with one of:
* for-of
* for-in
* array.map/array.filter for array-comprehension
as a part of bug 1083470.
converting rules are following:
* for-each
* for each (let x in array) { ... }
-> for (let x of array) { ... }
* for each (let x in object) { ... }
-> for (let key in object) { let x = object[key]; ... }
* for each (let [key, value] in Iterator(object)) { ... }
-> for (let key in object) { let value = object[key]; ... }
* for each (let x in array) { ... }
where array can be null or undefined
-> if (array) { for (let x of array) { ... } }
* legacy array comprehension with for-each
* [EXPR for each (x in array)]
-> array.map(x => EXPR)
* [EXPR for each (x in array) if (COND)]
-> array.filter(x => COND).map(x => EXPR)
* [x for each (x in array) if (COND)]
-> array.filter(x => COND)
* [EXPR for each ([i, x] in Iterator(array)) if (g(x, i)]
-> array.filter((x, i) => g(x, i)).map((x => EXPR)
* [EXPR for each (x in arraylike)]
-> Array.from(arraylike).map(x => EXPR)
* [EXPR for each (x in string)]
-> Array.prototype.slice.call(string).map(x => EXPR)
// Array.from behaves differently for surrogate-pair
I'll post a patch shortly.
Assignee | ||
Comment 1•9 years ago
|
||
Bug 1217082 - Remove for-each from toolkit/. r?Gijs
Attachment #8676981 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Comment 2•9 years ago
|
||
Updated•9 years ago
|
Attachment #8676981 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment 3•9 years ago
|
||
Comment on attachment 8676981 [details]
MozReview Request: Bug 1217082 - Remove for-each from toolkit/. r?Gijs
https://reviewboard.mozilla.org/r/22847/#review20385
::: toolkit/components/exthelper/extApplication.js:573
(Diff revision 1)
> + for (let id in this._cache) {
```
return Object.keys(this._cache).map(id => this._cache[id]));
```
::: toolkit/components/search/nsSearchService.js:4613
(Diff revision 1)
> + let engine = this._engines[name];
> engine = engine.wrappedJSObject;
Maybe just unify these into:
let engine = this.\_engines\[name\].wrappedJSObject;
::: toolkit/modules/PropertyListUtils.jsm:263
(Diff revision 1)
> - return [String.fromCharCode(c) for each (c in new Uint8Array(aBuffer, 0, 8))].
> + return Array.from(new Uint8Array(aBuffer, 0, 8)).map(c => String.fromCharCode(c)).
MDN says a Uint8Array has a map function, so the Array.from call seems redundant?
::: toolkit/modules/tests/xpcshell/test_propertyListsUtils.js:70
(Diff revision 1)
> - let dataAsString = [String.fromCharCode(b) for each (b in array[6])].join("");
> + let dataAsString = Array.from(array[6]).map(b => String.fromCharCode(b)).join("");
Likewise, I don't think Array.from() is necessary here.
::: toolkit/mozapps/extensions/test/xpcshell/head_addons.js:1381
(Diff revision 1)
> - else for each (let installList in gExpectedInstalls) {
> + else for (let id in gExpectedInstalls) {
Seeing as we're modifying this anyway, please remove the else.
Not only is "else for" peculiar syntax, else after return is unnecessary, so it can just go away. :-)
Assignee | ||
Comment 4•9 years ago
|
||
Thank you!
(In reply to :Gijs Kruitbosch from comment #3)
> ::: toolkit/modules/PropertyListUtils.jsm:263
> (Diff revision 1)
> > - return [String.fromCharCode(c) for each (c in new Uint8Array(aBuffer, 0, 8))].
> > + return Array.from(new Uint8Array(aBuffer, 0, 8)).map(c => String.fromCharCode(c)).
>
> MDN says a Uint8Array has a map function, so the Array.from call seems
> redundant?
>
> ::: toolkit/modules/tests/xpcshell/test_propertyListsUtils.js:70
> (Diff revision 1)
> > - let dataAsString = [String.fromCharCode(b) for each (b in array[6])].join("");
> > + let dataAsString = Array.from(array[6]).map(b => String.fromCharCode(b)).join("");
>
> Likewise, I don't think Array.from() is necessary here.
Uint8Array.map returns Uint8Array, so we cannot use it, as we're going to return Array of String ;)
Assignee | ||
Comment 5•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e188de86d867916ccc661fa1a9ba149974aba35
Bug 1217082 - Remove for-each from toolkit/. r=Gijs
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
You need to log in
before you can comment on or make changes to this bug.
Description
•