Growing with the Web

Migrating xterm.js from TSLint to ESLint

Published
Tags:

As you may know, TSLint is now deprecated in favor of ESLint with the typescript-eslint plugin. This post dives into how the migration went in xterm.js, hopefully it will help save some time for people searching for the same errors.

Automatic migration

I started the migration by installing eslint and the TS plugin:

yarn add --dev eslint @typescript-eslint/parser @typescript-eslint/eslint-plugin

Then used the tslint-to-eslint-config utility to automatically convert most rules:

npx tslint-to-eslint-config

This gave a basic .eslintrc to use as a starting point that used to tslint plugin for rules that could not be migrated, namely tslint-consistent-codestyle.

Project references aren’t supported

This is the first issue I hit, after trying to run eslint (eslint -c .eslintrc.js --ext .ts src/ addons/) there were many errors about files not being within the tsconfig.

/home/daimms/dev/Tyriar/xterm.js/addons/xterm-addon-webgl/src/renderLayer/Types.ts
  0:0  error  Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
The file does not match your project config: addons/xterm-addon-webgl/src/renderLayer/Types.ts.
The file must be included in at least one of the projects provided

This wasn’t true, it’s just the root tsconfig.json of xterm.js is a “solution” file that only points to other files, and this is yet to be supported. The fix for this right now is to enumerate all tsconfig projects as an array on the project property.

Before:

"parserOptions": {
  "project": "tsconfig.all.json",
  "sourceType": "module"
},

After:

"parserOptions": {
  "project": [
    "src/tsconfig.json",
    "src/browser/tsconfig.json",
    "src/common/tsconfig.json",
    "test/api/tsconfig.json",
    "test/benchmark/tsconfig.json",
    "addons/xterm-addon-attach/src/tsconfig.json",
    "addons/xterm-addon-fit/src/tsconfig.json",
    "addons/xterm-addon-search/src/tsconfig.json",
    "addons/xterm-addon-unicode11/src/tsconfig.json",
    "addons/xterm-addon-web-links/src/tsconfig.json",
    "addons/xterm-addon-webgl/src/tsconfig.json",
    "addons/xterm-addon-serialize/src/tsconfig.json",
    "addons/xterm-addon-serialize/benchmark/tsconfig.json"
  ],
  "sourceType": "module"
},

This is a shame that it’s needed for now as this list needs to include all transitive dependencies as well. Needing to reference the internal xterm-addon-serialize/benchmark project at the top level is something we explicitly wanted to avoid. There is advice to create a separate tsconfig.json just for eslint and use includes to include all your files in the v2 release but when I tried that Node ran out of memory.

Failing migrated rules

spaced-comment: TypeScript triple slash references not working

Rule

"spaced-comment": "error",

Error

10:1   error  Expected space or tab after '//' in comment                              spaced-comment

Code

/// <reference lib="dom"/>

The fix was to add / as an exception to the rule:

"spaced-comment": [
    "error",
    "always",
    { "markers": ["/"] }
],

@typescript-eslint/array-type: Error on ReadonlyArray

Rule

"@typescript-eslint/array-type": "error",

Error

583:23  error  Array type using 'Array<IMarker>' is forbidden. Use 'IMarker[]' instead  @typescript-eslint/array-type

Code

readonly markers: ReadonlyArray<IMarker>;

The fix I went with was to just require the generic way for readonly arrays only by changing the rule:

"@typescript-eslint/array-type": [
  "error",
  {
    "default": "array-simple",
    "readonly": "generic"
  }
]

Alternatively all references of ReadonlyArray<T> could be changed to readonly T[].

@typescript-eslint/member-delimiter-style: Semicolons causing problems

Rule

"@typescript-eslint/member-delimiter-style": [
    "error",
    {
        "multiline": {
            "delimiter": "semi",
            "requireLast": true
        },
        "singleline": {
            "delimiter": "semi",
            "requireLast": false
        }
    }
]

Error

641:33  error  Expected a semicolon  @typescript-eslint/member-delimiter-style

Code

onKey: IEvent<{ key: string, domEvent: KeyboardEvent }>;

The fix was to require the use of commas instead of semi-colons in single-line types:

"@typescript-eslint/member-delimiter-style": [
    "error",
    {
        "multiline": {
            "delimiter": "semi",
            "requireLast": true
        },
        "singleline": {
            "delimiter": "comma",
            "requireLast": false
        }
    }
]

@typescript-eslint/quotes: String must use singlequote

Rule

"@typescript-eslint/quotes": [
    "error",
    "single"
]

Error

36:23  error  Strings must use singlequote  @typescript-eslint/quotes

Code

await page.evaluate(`window.term.open(document.querySelector('#terminal-container'))`);

The fix was to allow this use of backticks even when not concatenating strings:

"@typescript-eslint/quotes": [
    "error",
    "single",
    { "allowTemplateLiterals": true }
],

Typings are not included in the project so eslint complains

Error

/home/daimms/dev/Tyriar/xterm.js/addons/xterm-addon-attach/typings/xterm-addon-attach.d.ts
  0:0  error  Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
The file does not match your project config: addons/xterm-addon-attach/typings/xterm-addon-attach.d.ts.
The file must be included in at least one of the projects provided

Unforatunately I couldn’t figure out how to get these files covered so I ignored them in the .eslintrc:

"ignorePatterns": "**/typings/*.d.ts"

Manually migrating tslint-consistent-codestyle

Most tslint-consistent-codestyle rules didn’t end up working as pointed out by this error:

Could not find implementations for the following rules specified in the configuration:
    naming-convention
    no-else-after-return
    prefer-const-enum
Try upgrading TSLint and/or ensuring that you have all necessary custom rules installed.
If TSLint was recently upgraded, you may have old rules configured which need to be cleaned up.

Each of these needed to be migrated manually. Additionally I wanted to remove tslint all together so there were some other rules as well.

no-else-after-return

There’s an almost drop in replacement for this built in:

no-else-return: [
  "error",
  { allowElseIf: false }
]

prefer-const-enum

This is currently a proposal so it could not be migrated.

naming-convention

This was a big one as there was a lot to the rules I had set up. This doesn’t migrate automatically since it’s a tslint plugin but luckily there is the naming-convention builtin for naming that is roughly equivalent.

Before:

"naming-convention": [
    true,
    { "type": "default", "format": "camelCase", "leadingUnderscore": "forbid" },
    { "type": "type", "format": "PascalCase" },
    { "type": "class", "format": "PascalCase" },
    { "type": "property", "modifiers": ["const"], "format": ["camelCase", "UPPER_CASE"] },
    { "type": "member", "modifiers": ["protected"], "format": "camelCase", "leadingUnderscore": "require" },
    { "type": "member", "modifiers": ["private"], "format": "camelCase", "leadingUnderscore": "require" },
    { "type": "variable", "modifiers": ["const"], "format": [ "camelCase", "UPPER_CASE"] },
    { "type": "variable", "modifiers": ["const", "export"], "filter": "^I.+Service$", "format": "PascalCase", "prefix": "I" },
    { "type": "interface", "prefix": "I" }
],

After:

"@typescript-eslint/naming-convention": [
    "error",
    { "selector": "default", "format": ["camelCase"] },
    // variableLike
    { "selector": "variable", "format": ["camelCase", "UPPER_CASE"] },
    { "selector": "variable", "filter": "^I.+Service$", "format": ["PascalCase"], "prefix": ["I"] },
    // memberLike
    { "selector": "memberLike", "modifiers": ["private"], "format": ["camelCase"], "leadingUnderscore": "require" },
    { "selector": "memberLike", "modifiers": ["protected"], "format": ["camelCase"], "leadingUnderscore": "require" },
    { "selector": "enumMember", "format": ["UPPER_CASE"] },
    // typeLike
    { "selector": "typeLike", "format": ["PascalCase"] },
    { "selector": "interface", "format": ["PascalCase"], "prefix": ["I"] },
],

typedef

Before:

"typedef": [
    true,
    "call-signature",
    "parameter"
],

After:

"@typescript-eslint/explicit-function-return-type": [
    "error",
    {
       "allowExpressions": true
    }
]

whitespace

Before:

"whitespace": [
    true,
    "check-branch",
    "check-decl",
    "check-module",
    "check-operator",
    "check-rest-spread",
    "check-separator",
    "check-type",
    "check-type-operator",
    "check-preblock"
]

After:

Most of the this was accomplished by adding a few rules, the main exception is that <T>this was no longer allowed due to the keyword-spacing rule, so I changed those to this as T.

"keyword-spacing": "error",
"no-irregular-whitespace": "error",
"no-trailing-spaces": "error",
"@typescript-eslint/type-annotation-spacing": "error",

New coverage

A bunch of formatting errors are now being caught that weren’t before, not sure why many of them weren’t working before.

@typescript-eslint/semi

Error

/home/daimms/dev/Tyriar/xterm.js/addons/xterm-addon-webgl/src/WebglRenderer.ts
  84:1  error  Expected indentation of 6 spaces but found 8  @typescript-eslint/indent

Code

    if (!this._gl) {
        throw new Error('WebGL2 not supported ' + this._gl);
    }

prefer-const

Error

/home/daimms/dev/Tyriar/xterm.js/src/Terminal.ts
  669:7   error  'pos' is never reassigned. Use 'const' instead  prefer-const

Code

      let pos;

      // get mouse coordinates
      pos = self._mouseService.getRawByteCoords(ev, self.screenElement, self.cols, self.rows);

@typescript-eslint/indent

90:1   error  Expected indentation of 2 spaces but found 3                  @typescript-eslint/indent
  /**
   * Triggers the onBinary event in the public API.
   * @param data The data that is being emitted.
   */
   triggerBinaryEvent(data: string): void;

@typescript-eslint/member-delimiter-style

There should be no space before the :

Error

641:33  error  Expected a semicolon  @typescript-eslint/member-delimiter-style

Code

hook(params: IParams) : void {}

Comments

comments powered by Disqus
Like this article?
Subscribe for more!