Ban rule identifies types in JSX now.

[dryganets]

PR checklist

• [X ] New feature, bugfix, or enhancement
  • [ X] Includes tests

Overview of change:

Currently, banType doesn't work as expected for JSX Elements.
This pull request fixes this problem and adds new tests.
I didn't update the documentation as I think this is a bug, not a new feature.

Is there anything you'd like reviewers to focus on?

CHANGELOG.md entry:

[bugfix] banType --> supports JSX elements.

[palantirtech]

Thanks for your interest in palantir/tslint, @dryganets! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

[adidahiya]

I like the idea, but I think this would be much more intuitive as a standalone rule in tslint-react. Users are unlikely to see the "ban types" rule and think it works for banning JSX elements. I'd be happy to look at a PR in that repo.

[dryganets]

Well, JSX element is a type in many cases. The tslint just have no enough semantic context to distinguish type from HTML tag in JSX as HTML tag is technically a type too.

I used the tags in order simplify the test case.

ability to ban the tag is just a silly side effect.

In real life, I'm using this rule to ban certain deprecated types inside of the JSX code.

Now it doesn't work out of the box. Well, you can't define the variable of the banned type but you still could do something like:

return (<MyBannedType> bla bla bla </MyBannedType>);

And this doesn't look good.

Do you suggest to make a separate rule for this still?

[adidahiya]

ability to ban the tag is just a silly side effect.

I don't understand... the only code change here has been to add checking for JSX opening & self-closing elements. So it seems like the PR is only adding support for banning tags.

If you want to do something aside from that, can you expand your test cases to illustrate what your goal is?

[dryganets]

In React/React native you could create a component which could be a class.

The JSX tag is a synthetic sugar around createClass( class ...)
The use case I'm trying to target is:

I have a ReactComponent (ts class):

class  MyBannedComponent extends React.Component {

}

Later on, it is always used as part of some JSX statement. You don't instantiate it manually usually. JSX generated code doing all the dirty work for you under the hood.

So code:

return (<MyBannedComponent/>);

actually creates and render the instance of the class MyBannedComponent which should be banned.

[adidahiya]

Right, but if your overall goal here is to ban usage of the component in JSX (I understand how JSX & React components work), then I still think it belongs as a new rule in tslint-react.

return (<MyBannedComponent />) is not using MyBannedComponent in a type position; it is using it in a value position in the TS syntax tree.

The ban-types rule specifically states:

Does not ban the corresponding runtime objects from being used.

So I think it's working as intended.

[dryganets]

ok.
palantir/tslint-react#137