Make the Iterator prototype have its own constructor#730
Make the Iterator prototype have its own constructor#730nikolaybotev wants to merge 2 commits intofacebook:mainfrom
Conversation
|
Hi @nikolaybotev! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
554363e to
232e545
Compare
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
This change makes the generator prototype chain fully conformant with
the specthe Iterator Helpers stage 3 proposal, and enables the following code to work when transpiled to older runtimes:Of course, the above already works, but since
Iterator.prototypecurrently returns the globalObjectprototype, installingforEachinstalls on all objects, which is not the intent, andRepeatIteratoris not able to function as anIterable, because it does not have the[Symbol.iterator]method. So, notably, with this PR, this will work as well:Also, note the above already works if regenerator is used in a standard configuration with preset-env in babel and core-js 3 is
required (which polyfills the Iterator constructor, and regenerator picks that up as the "native" Iterator prototype), however, regenerator is already almost fully compliant with the prototype chain spec for generators, and I figured it is good for completeness to add this here, as maybe there are cases where regenerator is not used in that combination with the latest core-js that supplies a proper Iterator constructor implementation.Unit tests have been added.
Addendum
I also went ahead and merged
GeneratorFunctionPrototypeintoGenerator, because the two were duplicates --Generatoris the prototype ofGeneratorFunction!GeneratorFunctionPrototypewas actually the one that was fully setup andGeneratorwas not referenced anywhere in the prototype+constructor chain, except it had its ownprototypeproperty set , which allowed it to be used in aninstanceoftest. That test looks atGenerator.prototypeonly, and because bothGenerator.prototypeandGeneratorFunctionPrototype.prototypewere set to the sameGeneratorPrototypeobject, bothGeneratorandGeneratorFunctionPrototypeare interchangeable in this position!In the graphic you can see GFP (
GeneratorFunctionPrototypein the code) as the object properly setup in the prototype chain andGeneratoris crossed out. You can think of the change as a two-step - 1) removeGenerator, 2) renameGeneratorFunctionPrototypetoGenerator.