-
Notifications
You must be signed in to change notification settings - Fork 76
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
Added new jsx-ban-elements rule to ban jsx elements #137
Added new jsx-ban-elements rule to ban jsx elements #137
Conversation
193c34c
to
7700599
Compare
@adidahiya, |
@dryganets did you enable CircleCI on your fork? https://circleci.com/projects |
7700599
to
631acb6
Compare
631acb6
to
fba4066
Compare
Thank you. |
package.json
Outdated
@@ -1,6 +1,6 @@ | |||
{ | |||
"name": "tslint-react", | |||
"version": "3.3.3", | |||
"version": "3.3.4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please don't bump the version in this commit. that will be done during the release process.
src/rules/jsxBanElementsRule.ts
Outdated
public static metadata: Lint.IRuleMetadata = { | ||
ruleName: "jsx-ban-elements", | ||
description: Lint.Utils.dedent` | ||
Bans specific jsx elements from being used.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "JSX", not "jsx"
src/rules/jsxBanElementsRule.ts
Outdated
} | ||
|
||
function parseOption([pattern, message]: [string, string | undefined]): IOption { | ||
return {message, pattern: new RegExp(`^${pattern}$`)}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we surround the configured pattern with ^$
? Why not leave that up to the user?
src/rules/jsxBanElementsRule.ts
Outdated
const typeName = node.tagName.getText(); | ||
for (const ban of ctx.options) { | ||
if (ban.pattern.test(typeName)) { | ||
ctx.addFailureAtNode(node, Rule.FAILURE_STRING_FACTORY(typeName, ban.message)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we highlight the failure on just the tag name rather than the whole tag expression? that way, if there are other lint errors in the tag expression, they're less likely to overlap, which makes it easier to read in your editor.
@@ -0,0 +1,7 @@ | |||
{ | |||
"rules": { | |||
"jsx-ban-elements": [true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the newer options syntax please.
"jsx-ban-elements": {
"options": [
["span", "Use div instead."]
]
}
Fixed all you asked. |
thanks! |
No description provided.