Closed Bug 968038 Opened 8 years ago Closed 5 years ago

Replace JS1.7 legacy generators with ES6 generators in Add-on SDK and chrome JS

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: Unfocused, Assigned: arai)

References

Details

Attachments

(1 file)

We use JS1.7 style generators all over the show. We really should be using ES6 generators, which have slightly different syntax and semantics. Because ES6 is the One True Path. And by that I mean ES6 generators are the universal standard that will get all the performance tweaks, meanwhile JS1.7 generators will eventually be deprecated.

*Not* calling this a good-first-bug, unless someone disagrees. Because, frankly, too few people are familiar enough with both types of generators :\
I wouldn't call it a good first bug, but you could probably mentor people to do per-directory patches (ie first browser/base, then browser/components/foo, bar, toolkit/content, etc.).
We should totally do that. I however fear that we will almost never finish this task as there probably are too many people still using the old syntax and writing new tests and code with it (me included if I don't remind myself every time).

Can we implement something that would somehow keep us from writing new code using legacy generator syntax? A push hook feels complicated and maybe a little too harsh...
For patches reviewed using our Review Board instance[1], we could write a Review Bot extension that looks for and opens issues for old generator syntax.

Just a possibility. :)

[1]: http://reviewboard.allizom.org/
So, first of all, confessing my lack of knowledge here, do we have documentation about the differences? Simple searches turn up things that look the same to me (at least at 1.30 in the morning): functions using the yield keyword to generate values. What are the particular differences that need refactoring? A document about this on MDN and a post to fx-team and m.d.platform would probably be a good start.
Old generators do return a value from gen.next() and throw StopIteration if they're out of values. New generators return {value: 123, done: false/true} from gen.next(). As long as we're using for..of to iterate over generator values we're good but if anything is using .next() then the transition might not be as easy.

It looks like there's quite a few places using .next() and fixing (or rather finding) the generator clients is a lot harder than just finding a function that contains a yield stmt.

And yes, our MDN article is unfortunately a little behind. We really need some updated docs on this.
Depends on: 977466
Blocks: 867617
Depends on: 1083476
Summary: Convert all generators from JS1.7 generators to ES6 generators → Replace JS1.7 legacy generators with ES6 generators in Add-on SDK and chrome JS
the attachment is a list of legacy generator's yield's position, collected by the following try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c5a5c3ca063b1a4a0477c52309adea1a254468e8

note that the position may be eval-ed code's line/column.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Depends on: 1321214
Depends on: 1321215
Depends on: 1321216
Depends on: 1321217
Depends on: 1321218
Depends on: 1321220
Depends on: 1321222
Depends on: 1321225
Depends on: 1321226
Depends on: 1321227
Depends on: 1321228
Depends on: 1321229
Depends on: 1321230
No longer depends on: 1083476
I'll close this bug after checking if there's remaining consumers.
Depends on: 1226398
Depends on: 1338248
Depends on: 1338250
Depends on: 1338251
Depends on: 1338253
Depends on: 1338254
Depends on: 1338255
Depends on: 1338257
Depends on: 1338258
Depends on: 1338262
Depends on: 1338263
Blocks: 1342144
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.