Skip to content

[no-unnecessary-type-assertion] false positive if referenced package has not been built #1973

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

Closed
dinofx opened this issue May 4, 2020 · 6 comments
Labels
awaiting response Issues waiting for a reply from the OP or another party package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin working as intended Issues that are closed as they are working as intended

Comments

@dinofx
Copy link
Contributor

dinofx commented May 4, 2020

This simple repo shows the problem:
https://github.com/dinofx/bug-typescript-eslint

A type from a referenced package is being cast to any. If the referenced package has not been compiled with TSC, a false positive is reported. For some reason linting thinks the type is already any, and that the cast is redundant.

Repro

Clone the repo and yarn install dependencies. Don't build, just open main.ts in VS Code with eslint extension.

Expected

Should be able to resolve types from referenced packages using their original *.ts source, rather than relying on missing or stale *.d.ts files.

@dinofx dinofx added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels May 4, 2020
@bradzacher
Copy link
Member

There's not a lot we can do about this.
We rely upon the types given to us by typescript itself.

If typescript cannot find a type, then it just reports the type as any - it provides no public signal to the consumer that it's an error.

cc @uniqueiniquity - I vaguely remember that vscode can now read the source files if a project (that's referenced in tsconfig) is unbuilt, is that something we should get "for free", or is there some work we need to do to make that work?

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party working as intended Issues that are closed as they are working as intended and removed triage Waiting for team members to take a look labels May 4, 2020
@dinofx
Copy link
Contributor Author

dinofx commented May 6, 2020

VS Code does not rely on the built declarations. Maybe this is relatively new.

In fact, VS Code never uses the .d.ts file (by default?). In fact, you can change a function in one "project", and you don't even need to save the file to see the changes reflected in references to that function from another "project".

@justinbhopper
Copy link

I have created another repo of the problem too, as I was running into false positives for the same reasons:

https://github.com/justinbhopper/eslint-issue

  1. Run yarn to install dependencies
  2. Run yarn lint, observe two false positive lint errors (require-await and restrict-plus-operands)
  3. Run yarn build to compile with tsc
  4. Run yarn lint, observe no errors
  5. Run yarn clean to remove compiled .d.ts files
  6. Run yarn lint, observe errors are back

This is a big problem for monorepos and means we end up having to disable ALL rules that require type checking. I think it would help if we could brainstorm on what information would be needed in order to solve the problem.

@bradzacher
Copy link
Member

VS Code does not rely on the built declarations.

Sorry, i think you misread my comment.
I said "vscode can now read the source files if a project (that's referenced in tsconfig) is unbuilt"

This is a "relatively" recent change.

we end up having to disable ALL rules that require type checking

I don't understand, why do you need to disable the rules? If you build the projects, it will work as expected.
We are a monorepo and we use type information rules.

I think it would help if we could brainstorm on what information would be needed in order to solve the problem.

TS provides many compiler APIs for different use cases. The API VSCode uses supports this. The API we use doesn't. We have specific needs which mean we cannot use a different API.

So we need support within typescript for this.
I've been talking offline to the TS team for a little while now about OOM issues and the problems we have juggling many projects.

They've been working on helping alleviate memory issues. There's a number of optimisations that are landing as part of 3.9.

Also, as I found out yesterday, to help with the OOM issues, they've already been working on bringing support to this API for uncompiled project references (which is similarly landing in 3.9)!
microsoft/TypeScript#37370

So what needs to happen? We need to now build out support within this project for the new features.

Unfortunately, the core, volunteer maintainers won't have time to do so for a month or more. So if it's important to you, we'd appreciate the help.

@uniqueiniquity is currently preparing a little guide for this work so that any community member can pitch in.

@justinbhopper
Copy link

I don't understand, why do you need to disable the rules? If you build the projects, it will work as expected.

@bradzacher Sorry I wasn't more clear - we have to disable the rules because they give false positives or false negatives during local development, because the compiled definitions end up stale.

The alternative is to only allow linting if a compile occurs first or we use tsc -w during development. Both of which makes no sense to me given that we've setup the typescript-eslint parser.

@bradzacher
Copy link
Member

Closing in favour of #2094

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 25, 2020
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 or another party package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin working as intended Issues that are closed as they are working as intended
Projects
None yet
Development

No branches or pull requests

3 participants