Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

no-unused-variable rule false positive on JSX elements in 3.4.0-dev.1 and TS 1.9.0-dev.20160216 #972

Closed
myitcv opened this issue Feb 16, 2016 · 12 comments

Comments

@myitcv
Copy link
Contributor

myitcv commented Feb 16, 2016

The following code (using tslint 3.4.0-dev.1 and tsc 1.9.0-dev.20160216) gives a false positive in no-unused-variable:

function BuildMyComponent(): JSX.Element {
    let StringComponentA = BuildComp();
    return <StringComponentA foo = { "test" } />;
}
[2, 6]: unused variable: 'StringComponentA'

If I find time later this week I can investigate further (unless anyone happens to beat me to it)

Note that in order to repro this against master you will need to npm install --save-dev typescript@next

On a related note, does it make sense to add a TypeScript nightly version to the Travis build matrix?

cc @adidahiya @jkillian

@adidahiya
Copy link
Contributor

We do build against typescript@next on the next branch, for example this PR. Does BuildComp() return a stateless function component?

@myitcv
Copy link
Contributor Author

myitcv commented Feb 16, 2016

We do build against typescript@next

@adidahiya my apologies, should have checked first.

Does BuildComp() return a stateless function component?

Hmm, yes all the instances where this is failing are stateless (at least I'm assuming stateless components are implicitly those with a state type of {}).

Have I missed something?

@adidahiya
Copy link
Contributor

SFCs are simple functions, not classes with constructor signatures. The compiler handles them a little differently. We currently don't have any tests for this new syntax. Either way, seems like a bug.

@myitcv
Copy link
Contributor Author

myitcv commented Feb 17, 2016

@adidahiya - I've reproduced this in TypeScript core and raised microsoft/TypeScript#7109

@jkillian
Copy link
Contributor

@myitcv Do you think #1010 and #1034 are the same issues as this?

@myitcv
Copy link
Contributor Author

myitcv commented Mar 17, 2016

@jkillian - potentially, but the bug that is listed in microsoft/TypeScript#7109 only appeared as of 1.9.0-dev.20160212

#1034 could well be another issue because they user reports v1.7.5

I've asked the question about version information in #1010

Incidentally, I think it would be worth using the new GitHub new issue template to encourage people to give us this sort of information up front...

@dallonf
Copy link

dallonf commented Mar 17, 2016

My ears are burning! Mentioned in detail in #1010, but worth repeating here: for me, this bug only shows up in Atom, and I believe the TSLint and TypeScript packages there track latest. So likely the same issue.

@myitcv
Copy link
Contributor Author

myitcv commented Mar 17, 2016

@dallonf - I had forgotten that atom-typescript behaves in this way. It's not helpful for sure... (@jkillian we could ask users whether they use Atom in the new issue template). Could well be the same issue therefore...

@jkillian
Copy link
Contributor

Took me a minute to see what you were saying @myitcv: For you @dallonf, Atom TypeScript isn't working right since it's basically using a newer version of TypeScript than you're using, and the bug was introduced between those two versions.

This is unfortunate for sure, it's much nicer when bugs are in older versions of TypeScript! Hopefully they can get this one fixed, bugs like this break reference finding and refactoring in IDEs, and IDE support is one major reason for using TS.

@myitcv
Copy link
Contributor Author

myitcv commented Mar 22, 2016

@dallonf @jkillian @adidahiya - per this comment, I believe this to have been fixed as of 1.9.0-dev.20160322

@myitcv myitcv closed this as completed Mar 22, 2016
@myitcv
Copy link
Contributor Author

myitcv commented Mar 22, 2016

@adidahiya @jkillian - I closed this issue because I've successfully tested this as no longer broken... but please feel free to re-open if you think this issue is capturing something else?

@jkillian
Copy link
Contributor

Thanks for the heads-up @myitcv! I tried it out as well and got the same results as you. Interestingly, #1034 also was fixed by this but not #1010 as far as I can tell.

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

No branches or pull requests

4 participants