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

Updated NodeType of TSTypeLiteral to TSLiteralType for typescript 2.9.x #137

Closed

Conversation

weikinhuang
Copy link

@weikinhuang weikinhuang commented Jun 26, 2018

After upgrading to [email protected] and [email protected] I've encountered an issue with running the typescript/no-type-alias rule against this file:

// Manually declaring because the official does not export classvalue
declare module 'classnames' {
  // Type definitions for classnames 2.2
  // Project: https://github.com/JedWatson/classnames
  // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped
  // TypeScript Version: 2.3
  export type ClassValue = string | number | ClassDictionary | ClassArray | undefined | null | false;

  interface ClassDictionary {
    [id: string]: boolean | undefined | null;
  }

  export interface ClassArray extends Array<ClassValue> { } // eslint-disable-line typescript/no-empty-interface
  type ClassNamesFn = (...classes: ClassValue[]) => string;

  const classNames: ClassNamesFn;

  export default classNames;
}

It would crash with:

Cannot read property '0' of undefined
TypeError: Cannot read property '0' of undefined
    at getMessage (/foobar/node_modules/eslint-plugin-typescript/lib/rules/no-type-alias.js:194:26)
    at validateTypeAliases (/foobar/node_modules/eslint-plugin-typescript/lib/rules/no-type-alias.js:280:30)
    at node.types.forEach.type (/foobar/node_modules/eslint-plugin-typescript/lib/rules/no-type-alias.js:296:21)
    at Array.forEach (<anonymous>)
    at validateCompositions (/foobar/node_modules/eslint-plugin-typescript/lib/rules/no-type-alias.js:292:24)
    at validateNode (/foobar/node_modules/eslint-plugin-typescript/lib/rules/no-type-alias.js:310:17)
    at VariableDeclaration (/foobar/node_modules/eslint-plugin-typescript/lib/rules/no-type-alias.js:322:21)
    at listeners.(anonymous function).forEach.listener (/foobar/node_modules/eslint/lib/util/safe-emitter.js:47:58)
    at Array.forEach (<anonymous>)
    at Object.emit (/foobar/node_modules/eslint/lib/util/safe-emitter.js:47:38)

At which point, I added this changeset, and some logging for the node, I got:

{ type: 'TSLiteralType',
  range: [ 388, 393 ],
  loc:
   { start: { line: 7, column: 95 },
     end: { line: 7, column: 100 } },
  transformFlags: undefined,
  literal:
   { type: 'Literal',
     range: [ 388, 393 ],
     loc: { start: [Object], end: [Object] },
     value: false,
     raw: 'false' } }

/foobar/types/classnames.d.ts
  7:96  error  Type aliases are not allowed  typescript/no-type-alias

@weikinhuang
Copy link
Author

I would appreciate some pointers on how to create a test case for this, as this only affects the newer version of typescript and the parser

@JamesHenry
Copy link
Collaborator

When you say it only affects the newer version, it is not a breaking change, right? If this change doesn't affect users on older versions then I don't think we need to stress about it. You could just add your source above as a test case

@weikinhuang
Copy link
Author

@JamesHenry Prior to the update,

export type ClassValue = string | number | ClassDictionary | ClassArray | undefined | null | false;

was not caught by the typescript/no-type-alias rule, and should probably continue to not to be. The configuration I'm using is:

        'typescript/no-type-alias': ['error', {
          allowAliases: 'in-unions-and-intersections',
          allowCallbacks: true,
          allowsLiterals: 'in-unions-and-intersections',
          allowMappedTypes: 'in-unions-and-intersections',
        }],

@scottohara
Copy link
Contributor

I have just upgraded one of my projects to [email protected] + [email protected].

I'm getting the same error as @weikinhuang:

TypeError: Cannot read property '0' of undefined
   at getMessage (/foobar/node_modules/eslint-plugin-typescript/lib/rules/no-type-alias.js:194:26)
   ...

for configuration:

"typescript/no-type-alias": ["error", {
  "allowAliases": "in-unions-and-intersections",
  "allowCallbacks": true,
  "allowsLiterals": "in-unions-and-intersections",
  "allowMappedTypes": "in-unions-and-intersections",
}]

and source:

export type <SomeType> = <union of types>

Looking at this patch, what is unclear (to me) is where upstream the change from TSTypeLiteral -> TSLiteralType occurred, that causes the error (and the need for this PR)?

My assumption was that it must have been a change in typescript-eslint-parser, but in that project I can find no references to TSLiteralType.

Has anyone identified yet if the change from TSTypeLiteral -> TSLiteralType was intentional?

@xt0rted
Copy link

xt0rted commented Sep 7, 2018

I just started setting up ESLint for my project and ran into this issue as well.

My package.json has the following versions in it

{
  "devDependencies": {
    "eslint": "5.5.0",
    "eslint-plugin-typescript": "0.12.0",
    "typescript": "3.0.3",
    "typescript-eslint-parser": "18.0.0"
  }
}

@bradzacher
Copy link
Owner

Hi @weikinhuang,
Could you please add some tests for your change?

@bradzacher bradzacher added the awaiting response Issues waiting for a reply from the OP label Nov 19, 2018
@armano2
Copy link
Contributor

armano2 commented Nov 19, 2018

@bradzacher TSLiteralType is not the same as TSTypeLiteral (name is confusing)

TSLiteralType is a string like type => alias

type foo = "BAR"

and this issue got fixed in f6041a6

@bradzacher
Copy link
Owner

Cool beans, thanks @armano2
Closing this as fixed.

@bradzacher bradzacher closed this Nov 20, 2018
@weikinhuang
Copy link
Author

Thanks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awaiting response Issues waiting for a reply from the OP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants