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

Confused by no-undef errors on type declarations #110

Closed
searls opened this issue Mar 4, 2018 · 22 comments
Closed

Confused by no-undef errors on type declarations #110

searls opened this issue Mar 4, 2018 · 22 comments

Comments

@searls
Copy link

searls commented Mar 4, 2018

I'm sure this is me doing it wrong, but I'm a bit confused about how to proceed. I'm invoking this plugin with standard.js via:

$ standard --fix --parser typescript-eslint-parser --plugin typescript "**/*.ts"

If my module looks like this:

import Call from '../value/call'
import Double from '../value/double'

export default interface Demonstration {
  call: Call
  double: Double
}

I get all of these errors:

  /Users/justin/code/testdouble/testdouble.js/src/value/demonstration.ts:4:26: 'Demonstration' is not defined.
  /Users/justin/code/testdouble/testdouble.js/src/value/demonstration.ts:5:3: 'call' is not defined.
  /Users/justin/code/testdouble/testdouble.js/src/value/demonstration.ts:6:3: 'double' is not defined.

I would have figured this plugin's job would be to override the eslint no-undef rule, but to be honest I'm just a bit confused by all the moving parts. Any thoughts would be appreciated.

@mightyiam
Copy link
Contributor

It's a known bug. As a workaround, just disable this rule. It is not necessary, anyway, because TypeScript guards you from that.

@searls
Copy link
Author

searls commented Mar 4, 2018

Ok, thanks. So I suppose that means there's just no practical way to lint typescript with standard (which isn't configurable)?

@mightyiam
Copy link
Contributor

@searls
Copy link
Author

searls commented Mar 4, 2018

I tried it and couldn't get it to work with the eslint cli before landing on this with the standard bin, actually ¯\(ツ)

@searls
Copy link
Author

searls commented Mar 4, 2018

(I didn't mean to come across as short, I just remember spending an hour trying to get that eslint configuration to work before giving up—if you know of a minimal example app that uses it I'd be willing to try again)

@searls
Copy link
Author

searls commented Mar 5, 2018

Thanks @mightyiam!

@macklinu
Copy link
Collaborator

@searls did you find a resolution to your issue?

@searls
Copy link
Author

searls commented Mar 26, 2018

Hi @macklinu I haven't had time to come back to this quite yet but I'm sure that @mightyiam's example projects will get me sorted. Closing

@searls searls closed this as completed Mar 26, 2018
@aboyton
Copy link
Contributor

aboyton commented Mar 26, 2018

My issue with disabling this rule and leaving TypeScript to enforce it is that TypeScript makes this a compilation error which is really annoying during development. Is there any way around this?

@mightyiam
Copy link
Contributor

Why exactly is it annoying during development? Does it have to do with an error message in the editor or something more?

@aboyton
Copy link
Contributor

aboyton commented Mar 27, 2018

I'm probably missing something obvious but if I set --noUnusedLocals in TypeScript's compiler options then it fails to compile if I have an unused variable which is really annoying when developing as I'm often adding and removing code and thus having unused variables all over the place that I'll clean up before I commit. I'm using Angular-CLI but I'm doubting that's making any difference.

@corbinu
Copy link
Contributor

corbinu commented Mar 27, 2018

@aboyton yes I guess we are just confused as to how that is more annoying than it failing to lint. You can always add a no output tsc to your lint script. Also you can add TypeScript checking to your editor just like you can add linting. For example VS Code does this automatically

@aboyton
Copy link
Contributor

aboyton commented Mar 27, 2018

So I realise that I'm wading into a discussion with a lot of opinions.

For me my current workflow is:

  • Use AngularCLI running in watch mode without --noUnusedLocals.
  • Edit in VSCode, constantly making changes and having them reflected in the browser.
  • When I'm finished, commit, make a diff with Phabricator (similar to making a pull request with GitHub) which then runs the linter and unit tests on my diff. There's nothing stopping me from making a diff with lint errors, they show up inline which can be useful for WIP code that I want someone to see.
  • When everyone is happy we merge the diff into master where CI then builds it and fails if the linting is broken.

This process works reasonable with TSLint's no-unused-variable rule (except that this rule is really buggy and appears to break other rules). I can run code with unused variables which personally I really like. Unfortunately VSCode doesn't allow me to use TSLint's no-unused-variable lint rule as it requires type information and so I don't see unused variables in VSCode (see TSLint's FAQs and https://github.com/Microsoft/vscode-tslint/issues/70. So sadly I only see my unused variables when I'm done and I make a Phabricator diff (which then runs TSLint) or I manually TSLint myself.

If I switch on --noUnusedLocals in TypeScript's compiler options then I do get VSCode showing me unused variables which is great, but I can no longer seem to compile my code with AngularCLI as this is an error not a warning. There's been a lot of discussion on microsoft/TypeScript#13408 about making these warnings not compile errors and if it's better to have the compiler or the linter enforcing things like this. Alex Eagle's comments microsoft/TypeScript#13408 (comment) and https://gist.github.com/alexeagle/349887ddb277aa5833643fc8c0b5465b are particularly interesting (to me at least). There's also a discussion about if linting should instead be moved to be part of a language service so you don't have to parse everything with type information twice (see microsoft/TypeScript#13408 (comment) and https://github.com/angelozerr/tslint-language-service).

@corbinu
Copy link
Contributor

corbinu commented Mar 27, 2018

@aboyton I would recommend using two tsconfigs one for lint and one for build

could set your tsconfig.json to have noUnusedLocals: true so that the errors show in VS Code then make your npm script something like this:

scripts: {
    "tsc": "tsc --noUnusedLocals=false",
    "lint": "eslint . && tsc --noEmit"

@aboyton
Copy link
Contributor

aboyton commented Mar 27, 2018

Thanks. I was wondering about that. I'll give it a try.

@mightyiam
Copy link
Contributor

Does AngularCLI have something like this for its TypeScript support?

@aboyton
Copy link
Contributor

aboyton commented Apr 15, 2018

Not act I've seen. This looks the most promising so far. microsoft/TypeScript#22361

@paulmelnikow
Copy link

Am I correct this issue is still outstanding? Could this be reopened until it's resolved?

I'm digging into some issues related to wmonk/create-react-app-typescript#354, and trying to sort out what to do here.

@mockdeep
Copy link

Running into this, too. We've got a mixed ts and js project as we convert things over, so would rather not disable the rule. Also, it's nice to have a couple layers of checking just in case.

@Offirmo
Copy link

Offirmo commented Sep 20, 2018

Running into this.

@mockdeep I believe you can selectively disable the rule only for .ts files using the "override" section of eslintrc

@mockdeep
Copy link

@Offirmo yeah, that's what we ended up doing:

overrides:
  - files: ['**/*.ts', '**/*.tsx'],
    parser: typescript-eslint-parser,
    plugins: [typescript],
    rules:
      no-undef: off

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants