Implement a "react" option for the no-unused-variable rule

[myitcv]

Addresses #698

"no-unused-variable", [true, "react"],

With such a valid option, the following is not an error:

import * as React from "react";

let x = <span></span>;
console.log(x);

Thoughts/comments?

[myitcv]

Rebased against master following merge of #726

[myitcv]

@jkillian @adidahiya just rebased this

[jkillian]

Ideally, it would be nicer to just accept one option named 'react' and not worry about prefixes. Especially since React 0.14 moved addons to their own npm packages (and hopefully bower packages?). Are there any other legit use cases other than 'react' and 'react/addons'?

[myitcv]

Are there any other legit use cases other than 'react' and 'react/addons'?

Not sure, hence why I coded a bit more defensively here... but on reflection it's much simpler to have the modules "react" and "react/addons" as constants which makes the option just "react".

I'll push up another commit that makes that change... then we can choose between the two (and then squash commits)

[adidahiya]

I'd prefer only supporting "react".

[jkillian]

Why not support both? I assume using "react/addons" is pretty common

[adidahiya]

Sorry, I should've been more clear --

I would like the rule option to be named "react". When looking for imports, we should look for ... from "react" and ... from "react/*".

[jkillian]

Oh gotchya. I was thinking have the option be named "react" only and when looking for imports only look for "react" and "react/addons".

[majo-javorka]

hi folks. I just found this discussion as I saw the same thing in my project. I also have "unused variable: 'defaultProps'", do you think it would make sense to 'whitelist' that as well when "react" option is set?

[jkillian]

@Jazzik Can you post an example of your code? I believe as long as it's not private, you shouldn't get that error. Something like this shouldn't give a lint error:

    interface Props { foo: number; }
    export class ReactComponent extends React.Component<Props, {}> {
        public static defaultProps = {
            foo: 12
        };
        ...
    }

[majo-javorka]

@jkillian oh right, I had it as private 😊

[myitcv]

@jkillian @adidahiya pushed up a second commit with a single "react" option. The list of valid React modules is now maintained as a const in the rule.

[myitcv]

@adidahiya @jkillian - any thoughts on this? Only reason for me pushing is that we've got a whole load of /* tslint:disable:no-unused-variable */ lines kicking around our code which I'm keen to remove 😄

[jkillian]

Can these two consts not start with OPTION_? I think that should be reserved only for actual options which are provided to the rule

[adidahiya]

change this to "... a variable usage failure will ..."

"error" is ambiguous in JS, so we should stick to the "failure" terminology.

[myitcv]

@adidahiya - all points addressed in most recent push. Apologies, a number of the whitespace changes crept in because my editor automatically runs tsfmt on save. Look forward to #732 landing and being enforced in the build 😄