Add support for @@iterator #7058

rajasekarm · 2017-12-19T10:45:17Z

Q	A
Fixed Issues?	Fixes #7053
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?	Yes
Tests Added + Pass?	Yes
Documentation PR
Any Dependency Changes?
License	MIT

babel-bot · 2017-12-19T10:54:00Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/6584/

nicolo-ribaudo

There is also @@asyncIterator: https://github.com/facebook/flow/blob/4eb62a5597d59174531d44e532c65daa25d8331b/src/parser/lexer.ml#L805-L807
Should the @@iterator identifier be allowed everywhere? e.g. let @@iterator = 2;

nicolo-ribaudo · 2017-12-21T20:46:49Z

packages/babylon/src/tokenizer/index.js

@@ -1231,6 +1231,9 @@ export default class Tokenizer extends LocationParser {
      const ch = this.fullCharCodeAtPos();
      if (isIdentifierChar(ch)) {
        this.state.pos += ch <= 0xffff ? 1 : 2;
+      } else if (this.state.isIterator) {
+        this.state.pos += 2;
+        delete this.state.isIterator;


Can you use this.state.isIterator here?

I meant this.state.isIterator = false lol

hzoo · 2017-12-21T21:11:56Z

@nicolo-ribaudo it's a flow syntax not related to JS, it would error with let @@iterator = 1? Also funny since the link to flow says TODO switch with Symbol.iterator?

nicolo-ribaudo · 2017-12-21T21:30:51Z

Also funny since the link to flow says TODO switch with Symbol.iterator?

We should ask to the flow team if they plan to remove @@iterator in the near future.

hzoo · 2017-12-21T23:13:22Z

Should ping the flow team on all the flow prs anyway 😛 @babel/flow

rajasekarm · 2017-12-22T07:11:49Z

Also we should not allow any other identifier iterator and asyncIterator. I'll update the PR

rajasekarm · 2017-12-22T14:26:40Z

@nicolo-ribaudo

Should the @@iterator identifier be allowed everywhere? e.g. let @@iterator = 2;

Flow parse the above syntax, but in V8 throws an error.

nicolo-ribaudo · 2017-12-22T14:35:43Z

Flow parse the above syntax

I checked and flow paeses that syntax sometimes (e.g. it accepts let @@iterator = number but not type @@iterator = number).
I think it's ok for us to allow it only in class/object/interface property types.

nicolo-ribaudo · 2017-12-30T23:11:37Z

I have thought about this a little, and I think that we should allow @@iterator only in class/object/interface.
The flow parser is more lenient and accepts things like

function @@iterator() {}
@@iterator();

but I think that we shouldn't parse it since it will generate invalid JavaScript output.

So, instead of throwing if @@iterator is after var/let/type we should throw if it is not inside an object/interface/class property name.

existentialism · 2018-01-03T02:45:23Z

@nicolo-ribaudo agreed

loganfsmyth · 2018-01-09T05:38:22Z

@danez Would you like to look this over since you're familiar with the parser? It seems fine to me, but I'm also not an expert on it.

danez

Overall looks good to me, but there are some remaining issues and untested things.

Flow-strip-types probably needs to remove the @@iterator and @@asyncIterator, as the resulting code is otherwise invalid?
Does babel-generator know how to reprint the examples which are added in the babylon tests? Can we add a simple test for this? (We should really have shared testcases for babylon and generator at some point, as everything that is parsable should be printable I guess)

nicolo-ribaudo · 2018-01-09T10:40:07Z

Flow-strip-types probably needs to remove the @@iterator and @@asyncIterator, as the resulting code is otherwise invalid?

Currently Flow only removes them from types.

xtuc · 2018-01-09T10:58:06Z

Looks good to me. As mentioned by @danez, babel-generator will probably need to be updated in order to emit this syntax and could you please add a test for it?

It could probably also impact TypeScript? @hzoo could we ping them? I don't think that could break anything but maybe require a transformations (I'm not a TS expert).

nicolo-ribaudo

actual/expected -> input/output in tests 😛

rajasekarm · 2018-01-09T18:32:52Z

@nicolo-ribaudo updated the test cases.

rajasekarm · 2018-01-10T01:01:12Z

@danez I added test cases, to make sure iterator is removed from types in Flow-strip-types. 36a0731, Do you want to add test case for babel-generator, since it is already removed by Flow-strip-types

nicolo-ribaudo · 2018-01-10T06:05:33Z

Yes, since someone might use Babel for a codemod and thus not remove Flow types.

rajasekarm · 2018-01-11T03:32:55Z

@nicolo-ribaudo @danez added test case for generator and Flow-strip-types plugin

nicolo-ribaudo · 2018-01-11T18:03:03Z

...es/babel-plugin-transform-flow-strip-types/test/fixtures/strip-types/strip-iterator/input.js

@@ -0,0 +1,15 @@
+declare class A {


It looks like the output for this file is missing?

That's expected (i.e. there is no output), similar to https://github.com/babel/babel/tree/master/packages/babel-plugin-transform-flow-strip-types/test/fixtures/strip-types/strip-declare-statements

@existentialism Thanks!!

existentialism · 2018-01-12T15:19:17Z

@rajzshkr thanks!

nicolo-ribaudo reviewed Dec 21, 2017

View reviewed changes

existentialism added pkg: parser PR: Bug Fix 🐛 A type of pull request used for our changelog categories labels Jan 3, 2018

danez suggested changes Jan 9, 2018

View reviewed changes

nicolo-ribaudo approved these changes Jan 9, 2018

View reviewed changes

nicolo-ribaudo requested changes Jan 9, 2018

View reviewed changes

rajasekarm added 8 commits January 11, 2018 08:23

Add support for @@iterator

4a6ddb9

Add support for @@iterator

d037bc2

iterator and asyncIterator enable in flow with test cases

922ad26

iterator and asyncIterator enable in flow with test cases

b07f114

review comments

589cc66

changed test file names to input/output

e27d0b4

remove stripe types

ce11622

added test case for generator

058abf6

nicolo-ribaudo reviewed Jan 11, 2018

View reviewed changes

nicolo-ribaudo approved these changes Jan 11, 2018

View reviewed changes

danez approved these changes Jan 12, 2018

View reviewed changes

xtuc approved these changes Jan 12, 2018

View reviewed changes

existentialism merged commit 2d05487 into babel:master Jan 12, 2018

rajasekarm deleted the bug-7053 branch January 12, 2018 15:33

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 5, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for @@iterator #7058

Add support for @@iterator #7058

rajasekarm commented Dec 19, 2017

babel-bot commented Dec 19, 2017 •

edited

Loading

nicolo-ribaudo left a comment

nicolo-ribaudo Dec 21, 2017

nicolo-ribaudo Dec 21, 2017

hzoo commented Dec 21, 2017

nicolo-ribaudo commented Dec 21, 2017

hzoo commented Dec 21, 2017 •

edited

Loading

rajasekarm commented Dec 22, 2017

rajasekarm commented Dec 22, 2017 •

edited

Loading

nicolo-ribaudo commented Dec 22, 2017 •

edited

Loading

nicolo-ribaudo commented Dec 30, 2017

existentialism commented Jan 3, 2018

loganfsmyth commented Jan 9, 2018

danez left a comment •

edited

Loading

nicolo-ribaudo commented Jan 9, 2018

xtuc commented Jan 9, 2018

nicolo-ribaudo left a comment

rajasekarm commented Jan 9, 2018

rajasekarm commented Jan 10, 2018

nicolo-ribaudo commented Jan 10, 2018

rajasekarm commented Jan 11, 2018

nicolo-ribaudo Jan 11, 2018

existentialism Jan 11, 2018 •

edited

Loading

rajasekarm Jan 12, 2018

existentialism commented Jan 12, 2018

Add support for @@iterator #7058

Add support for @@iterator #7058

Conversation

rajasekarm commented Dec 19, 2017

babel-bot commented Dec 19, 2017 • edited Loading

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

nicolo-ribaudo Dec 21, 2017

Choose a reason for hiding this comment

nicolo-ribaudo Dec 21, 2017

Choose a reason for hiding this comment

hzoo commented Dec 21, 2017

nicolo-ribaudo commented Dec 21, 2017

hzoo commented Dec 21, 2017 • edited Loading

rajasekarm commented Dec 22, 2017

rajasekarm commented Dec 22, 2017 • edited Loading

nicolo-ribaudo commented Dec 22, 2017 • edited Loading

nicolo-ribaudo commented Dec 30, 2017

existentialism commented Jan 3, 2018

loganfsmyth commented Jan 9, 2018

danez left a comment • edited Loading

Choose a reason for hiding this comment

nicolo-ribaudo commented Jan 9, 2018

xtuc commented Jan 9, 2018

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

rajasekarm commented Jan 9, 2018

rajasekarm commented Jan 10, 2018

nicolo-ribaudo commented Jan 10, 2018

rajasekarm commented Jan 11, 2018

nicolo-ribaudo Jan 11, 2018

Choose a reason for hiding this comment

existentialism Jan 11, 2018 • edited Loading

Choose a reason for hiding this comment

rajasekarm Jan 12, 2018

Choose a reason for hiding this comment

existentialism commented Jan 12, 2018

babel-bot commented Dec 19, 2017 •

edited

Loading

hzoo commented Dec 21, 2017 •

edited

Loading

rajasekarm commented Dec 22, 2017 •

edited

Loading

nicolo-ribaudo commented Dec 22, 2017 •

edited

Loading

danez left a comment •

edited

Loading

existentialism Jan 11, 2018 •

edited

Loading