Skip to content
This repository was archived by the owner on Jan 19, 2019. It is now read-only.

Definite assignment assertions not in AST #424

Closed
azz opened this issue Dec 24, 2017 · 4 comments
Closed

Definite assignment assertions not in AST #424

azz opened this issue Dec 24, 2017 · 4 comments

Comments

@azz
Copy link
Contributor

azz commented Dec 24, 2017

What version of TypeScript are you using?
typescript@2.7.0-dev.20171224

What version of typescript-eslint-parser are you using?
master

What code were you trying to parse?

class X { a!: any }

What did you expect to happen?
Something like definite: true or exclamationToken: true in the AST. In the TS AST it is called exclamationToken.

New in microsoft/TypeScript#20166

What happened?

{
  type: 'ClassProperty',
  range: [ 10, 17 ],
  loc: {...},
  key:  {...},
  value: null,
  computed: false,
  static: false,
  readonly: undefined,
  typeAnnotation:  {...}
}

Thoughts on which property name to go for?

@sgoll
Copy link

sgoll commented Jan 9, 2018

I'd go with exclamationToken for two reasons:

  1. The parser should IMHO add as little additional interpretation (i.e. semantics) as possible, and for now the exclamation mark token might mean "definite" but this may change or broaden at some later point.
  2. The TS AST calls it exclamationToken and sticking to the same name avoids confusion.

azz added a commit to azz/typescript-eslint-parser that referenced this issue Jan 11, 2018
azz added a commit to azz/typescript-eslint-parser that referenced this issue Jan 11, 2018
azz added a commit to azz/typescript-eslint-parser that referenced this issue Jan 11, 2018
@JamesHenry
Copy link
Member

For me the parallel with optional (coming from questionToken in the original TS AST) is very strong.

As per Nicolo's comment here: babel/babel#7159 (comment)

@JamesHenry
Copy link
Member

@sgoll We will definitely be going for whatever is agreed on that babylon PR to stay aligned, so if you feel strongly about it either way please add to the discussion on there. Thanks!

@sgoll
Copy link

sgoll commented Jan 12, 2018

@JamesHenry Done that, and you are right, of course—the parallel with optional is strong.

azz added a commit to azz/typescript-eslint-parser that referenced this issue Feb 10, 2018
azz added a commit to azz/typescript-eslint-parser that referenced this issue Feb 17, 2018
azz added a commit to azz/typescript-eslint-parser that referenced this issue Feb 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants