Skip to content

Use discriminated unions to "branch" types for Property Names #1345

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

Closed
bradzacher opened this issue Dec 18, 2019 · 0 comments · Fixed by #1349
Closed

Use discriminated unions to "branch" types for Property Names #1345

bradzacher opened this issue Dec 18, 2019 · 0 comments · Fixed by #1349
Labels
enhancement New feature or request package: typescript-estree Issues related to @typescript-eslint/typescript-estree

Comments

@bradzacher
Copy link
Member

Types for things like ClassProperty/MethodDefinition/Property, etc are pretty ugly.

If the property name is computed, then any Expression is valid, otherwise only Identifier | Literal is valid.

However we don't represent this in the types, we just have a single object with key: Expression.

It'll be a bit of extra work, but we can improve upon this by doing something like:

type PropertyNameNonComputed = Identifier | Literal;
type PropertyNameComputed = Expression;

interface ClassPropertyBase extends BaseNode {
  type: AST_NODE_TYPES.ClassProperty;
  key: PropertyNameNonComputed | PropertyNameComputed;
  computed: boolean;
  // ... other props
}

interface ClassPropertyNonComputedKey extends ClassPropertyBase {
  key: PropertyNameNonComputed;
  computed: false;
}

interface ClassPropertyComputedKey extends ClassPropertyBase {
  key: PropertyNameComputed;
  computed: true;
}

type ClassProperty = ClassPropertyNonComputedKey | ClassPropertyComputedKey;

Nodes we should do this for:

  • ClassProperty
  • TSAbstractClassProperty
  • MethodDefinition
  • TSAbstractMethodDefinition
  • Property
  • TSPropertySignature
  • TSMethodSignature
  • MemberExpression
  • OptionalMemberExpression
@bradzacher bradzacher added enhancement New feature or request package: typescript-estree Issues related to @typescript-eslint/typescript-estree labels Dec 18, 2019
@bradzacher bradzacher changed the title Use discriminated unions to "branch" types and narrow types in certain cases Use discriminated unions to "branch" types for Property Names Dec 18, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request package: typescript-estree Issues related to @typescript-eslint/typescript-estree
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant