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

aegir lint complaints about imports in ts files #618

Closed
Gozala opened this issue Aug 7, 2020 · 4 comments · Fixed by #621
Closed

aegir lint complaints about imports in ts files #618

Gozala opened this issue Aug 7, 2020 · 4 comments · Fixed by #621

Comments

@Gozala
Copy link
Contributor

Gozala commented Aug 7, 2020

Describe the bug

As the following pull request illustrates multiformats/js-multiaddr#131 aegir lint complains about imports in ts files.

As far as I can tell that happens because it lints .ts files

aegir/src/lint.js

Lines 9 to 20 in 2fe4403

const FILES = [
'*.{js,ts}',
'bin/**',
'config/**/*.{js,ts}',
'test/**/*.{js,ts}',
'src/**/*.{js,ts}',
'tasks/**/*.{js,ts}',
'benchmarks/**/*.{js,ts}',
'utils/**/*.{js,ts}',
'!**/node_modules/**',
'!**/*.d.ts'
]

With JS configuration unless global ts option is used
https://github.com/ipfs/aegir/blob/master/src/lint.js#L86

This creates problems in cases where project combination of .ts and .js files.

I'm not too familiar with ESLint, but as far as I can tell there's no way to specify different parseOptions per file type

parserOptions: {
sourceType: 'module'
},

@Gozala
Copy link
Contributor Author

Gozala commented Aug 7, 2020

@hugomrdias could we leave out .ts files from eslint and let the TSC do the linting there ? I think TS supports plenty of rules to disallow unused vars, params, implicit this, return, etc...

@Gozala
Copy link
Contributor Author

Gozala commented Aug 7, 2020

Actually it looks like overrides could be used to provide different rules based on file type https://eslint.org/docs/user-guide/configuring#specifying-processor

@Gozala
Copy link
Contributor Author

Gozala commented Aug 7, 2020

Following local .eslintrc seems to resolve aegir lint problem:

{
  "overrides": [
    {
      "files": [
        "**/*.ts"
      ],
      "extends": "./node_modules/aegir/src/config/eslintrc-ts.js"
    },
    {
      "files": [
        "**/*.js"
      ],
      "extends": "./node_modules/aegir/src/config/eslintrc.js"
    }
  ]
}

But now I see problem when running tests

aegir test                                 ✗ ✱
Test Node.js
Warning: Cannot find any files matching pattern "test/node.{js,ts}"

/Users/gozala/Projects/js-multiaddr/test/types.spec.ts:1
import Multiaddr from "../src";
^^^^^^

SyntaxError: Cannot use import statement outside a module
    at wrapSafe (internal/modules/cjs/loader.js:1054:16)
    at Module._compile (internal/modules/cjs/loader.js:1102:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1158:10)
    at Module.load (internal/modules/cjs/loader.js:986:32)
    at Function.Module._load (internal/modules/cjs/loader.js:879:14)
    at Module.require (internal/modules/cjs/loader.js:1026:19)
    at require (internal/modules/cjs/helpers.js:72:18)
    at Object.exports.requireOrImport (/Users/gozala/Projects/js-multiaddr/node_modules/mocha/lib/esm-utils.js:20:12)
    at Object.exports.loadFilesAsync (/Users/gozala/Projects/js-multiaddr/node_modules/mocha/lib/esm-utils.js:33:34)
    at async singleRun (/Users/gozala/Projects/js-multiaddr/node_modules/mocha/lib/cli/run-helpers.js:156:3)
    at async Object.exports.handler (/Users/gozala/Projects/js-multiaddr/node_modules/mocha/lib/cli/run.js:366:5)
Command failed with exit code 1: mocha --ui bdd --timeout 5000 --unhandled-rejections=strict --colors --exit test/node.{js,ts} test/**/*.spec.{js,ts}

Not sure what is causing this one.

@Gozala
Copy link
Contributor Author

Gozala commented Aug 7, 2020

I am starting to think that maybe *.ts files should only be included if --ts is passed. Otherwise a lot of problems just appear due to having .ts files in the repo without opting to even check them.

achingbrain pushed a commit that referenced this issue Aug 14, 2020
Without this change `aegir` either uses esmodule parse rules or script based parse rules, which causes issues with repos that contain both `.ts` and `.js` files regardless of `--ts` option. Without `--ts` option it complains about `import` statements with `--ts` option it complains about use of unnecessary `'use strict'`.

This change introduces config for eslint that overrides based on file type using js config for js files and ts config for ts files. It also adds a test case which fails without this change.

Fixes #618

BREAKING CHANGE: new linting file used for .ts files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant