Use ESlint instead of TSlint #4901
Conversation
|
Not relevant anymore. |
| @@ -45,14 +44,15 @@ const project: TransformCompiler = { | |||
| const parsed: Dict<SelectionProjection> = {}; | |||
| const timeUnits: Dict<TimeUnitComponent> = {}; | |||
|
|
|||
| const signals = {}; | |||
| const signals = new Set<string>(); | |||
There was a problem hiding this comment.
I changed the code here so we don't do an assignment in the return.
| @@ -249,27 +249,6 @@ export class UnitModel extends ModelWithField { | |||
| return this.encoding; | |||
| } | |||
|
|
|||
| public toSpec(excludeConfig?: any, excludeData?: any) { | |||
There was a problem hiding this comment.
Are you sure it is unused across projects?
There was a problem hiding this comment.
I think compassql might use it but I think then it would make sense to move it there. https://github.com/search?q=org%3Avega+toSpec&type=Code
| @@ -0,0 +1,321 @@ | |||
| import {AggregateOp} from 'vega'; | |||
There was a problem hiding this comment.
This makes the namespace cleaner.
test/compile/scale/type.test.ts
Outdated
| ['bar', 'rule', 'rect'].forEach(mark => { | ||
| expect(scaleType({}, channel, {type: t}, 'rect', defaultScaleConfig)).toEqual(ScaleType.BAND); | ||
| [BAR, RULE, RECT].forEach(mark => { | ||
| expect(scaleType({}, channel, {type: t}, mark, defaultScaleConfig)).toEqual(ScaleType.BAND); |
There was a problem hiding this comment.
This was a bug I found through eslint.
|
This migration makes sense at some point. The question is whether the TS support in ESLint is good enough yet? |
|
Support seems pretty good. You can see https://github.com/typescript-eslint/typescript-eslint and in particular the list of rules at https://github.com/typescript-eslint/typescript-eslint/tree/master/packages/eslint-plugin#supported-rules |
|
Not sure why the PR build fails. It says it can't find tslint but we don't need it anymore. @travis is behaving oddly here, I think. |
|
I moved Embed over to Eslint. |
|
Btw, if you have time to mess with this, please revert tsc project between src and test first. |
|
@kanitw I reverted the ts project references. Once it's in, can we merge this (I can merge master into this then)? |
|
Merged master |
TSLint is deprecated (https://medium.com/palantir/tslint-in-2019-1a144c2317a9) and we already use ESLint in Vega. This makes things consistent.
Once this is in, we can also start using recommended rule sets such as the one from airbnb (https://github.com/airbnb/javascript/tree/master/packages/eslint-config-airbnb).