Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider handling typescript-eslint-specific AST #71

Closed
vhfmag opened this issue Feb 6, 2019 · 2 comments · Fixed by #72
Closed

Consider handling typescript-eslint-specific AST #71

vhfmag opened this issue Feb 6, 2019 · 2 comments · Fixed by #72

Comments

@vhfmag
Copy link
Contributor

vhfmag commented Feb 6, 2019

The team I work on is migrating a Flow project to Typescript, and we are trying to keep our ESLint setup by using typescript-eslint, but things were not working out as expected.

After some debugging, we found out that the issue was within jsx-ast-utils, which didn't correctly handle JSX attributes in the form of atribute={value!} (where ! is a Typescript non-null assertion, i.e. it tells Typescript to assume that value is neither null or undefined). It crashed here, more specifically. The issue is that @typescript-eslint/typescript-estree exposes it as a TSNonNullExpression node with value's AST as node.expression.

For our codebase, simply adding

if (type === "TSNonNullExpression") {
  expression = expression.expression;
  type = expression.type;
}

before that check made everything work. It's a fairly large React codebase, so I guess it's fair to assume there would be little additional work.

I would be happy to come up with a PR if the project sees this as desirable. Thanks!

@ljharb
Copy link
Member

ljharb commented Feb 6, 2019

Seems desirable to me.

@vhfmag
Copy link
Contributor Author

vhfmag commented Feb 6, 2019

Just submitted #72 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants