Skip to content

Fix Typography lint catching non truthy props #1137

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jan 28, 2021
Merged

Fix Typography lint catching non truthy props #1137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0b85a8e
Fix Typography lint catching non truthy props
M-i-k-e-l Jan 14, 2021
72ec0c3
Check for relevant components instead of value
M-i-k-e-l Jan 26, 2021
fc5c00a
Merge branch 'master' into Infra/fix-typography-lint
ethanshar Jan 27, 2021
3d74107
Merge branch 'master' into Infra/fix-typography-lint
M-i-k-e-l Jan 27, 2021
283cd9a
Add comments
M-i-k-e-l Jan 27, 2021
ef30ba2
Merge branch 'master' into Infra/fix-typography-lint
M-i-k-e-l Jan 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion eslint-rules/lib/rules/typography-deprecation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,21 @@ module.exports = {
localImportSpecifier = utils.getLocalImportSpecifier(node, source, defaultImportName);
}

function isComponentRelevant(node, components) {
let isComponentRelevant = true;
if (!_.isEmpty(components)) {
if (_.get(node, 'parent.type') === 'JSXOpeningElement') {
return components.includes(_.get(node, 'parent.name.name'));
}
}

return isComponentRelevant;
}

function findAndReportDeprecation(node, possibleDeprecation, useShortVersion) {
const path = `${defaultImportName}.${possibleDeprecation}`;
const foundDeprecation = _.find(deprecations, {path});
if (foundDeprecation) {
if (foundDeprecation && isComponentRelevant(node, foundDeprecation.components)) {
reportDeprecatedTypography(node, foundDeprecation, useShortVersion);
}
}
Expand Down
2 changes: 1 addition & 1 deletion eslint-rules/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "eslint-plugin-uilib",
"version": "1.0.27",
"version": "1.0.28",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That means that if we publish this version, it'll be used in production.
Are we ok with that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so (does hoping really hard helps? 🤞 ).
Do you have any reservations?

"description": "uilib set of eslint rules",
"keywords": [
"eslint",
Expand Down
272 changes: 271 additions & 1 deletion eslint-rules/tests/lib/rules/typography-deprecation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,86 @@ ruleTester.run('typography-deprecation', rule, {
{
options: options,
code: `${fullClassValidRenamed}`,
},
{
options: options,
code: `
${ourImport}
import {List} from 'another-source';
<List.Item title={'bla'} />`
},
{
options: options,
code: `
import React, {Component} from 'react';
import {Typography} from 'our-source';
import {List} from 'another-source';
export default class OurList extends Component {
render() {
const titleVal = 'bla';
return (
<List.Item title={titleVal}/>
)
}
}`
},
{
options: options,
code: `
import React, {Component} from 'react';
import {Typography, List} from 'our-source';
export default class OurList extends Component {
render() {
const titleVal = 'bla';
return (
<List.Item title={titleVal}/>
)
}
}`
},
{
options: options,
code: `
import React, {Component} from 'react';
import {Typography, List} from 'our-source';
export default class OurList extends Component {
render() {
const titleVal = this.props.title;
const subtitleVal = this.props.subtitle;
return (
<List.Item title={titleVal} subtitle={subtitleVal}/>
)
}
}`
},
{
options: options,
code: `
import React, {Component} from 'react';
import {Typography, List} from 'our-source';
export default class OurList extends Component {
render() {
const {title, subtitle} = this.props;
return (
<List.Item title={title} subtitle={subtitle}/>
)
}
}`
},
{
options: options,
code: `
import React, {Component} from 'react';
import {Typography, List} from 'our-source';
export default class OurList extends Component {
render() {
const {title: titleVal, subtitle: subtitleVal} = this.props;
return (
<List.Item title={titleVal} subtitle={subtitleVal}/>
)
}
}`,
errors: [{message: `'Typography.title' is deprecated. Please use 'Typography.heading' instead (fix is available).`}]
}
],
invalid: [
Expand Down Expand Up @@ -439,6 +519,196 @@ ruleTester.run('typography-deprecation', rule, {
options: options,
code: `${fullClassTest2}`,
errors: [{message: error}]
}
},
{
options: options,
code: `
import React, {Component} from 'react';
import {Typography, Text} from 'our-source';
export default class OurList extends Component {
render() {
const titleVal = true;
return (
<Text title={titleVal}/>
)
}
}`,
errors: [{message: `'Typography.title' is deprecated. Please use 'Typography.heading' instead (fix is available).`}]
},
{
options: options,
code: `
import React, {Component} from 'react';
import {Typography, Text} from 'our-source';
export default class OurList extends Component {
render() {
const {isTitle} = this.props;
const titleVal = this.props.isTitle;
const subtitleVal = !this.props.isTitle;
return (
<Text title={titleVal} subtitle={subtitleVal}/>
)
}
}`,
errors: [{message: `'Typography.title' is deprecated. Please use 'Typography.heading' instead (fix is available).`}]
},
{
options: options,
code: `
import React, {Component} from 'react';
import {Typography, Text} from 'our-source';
export default class OurList extends Component {
render() {
const {isTitle} = this.props;
const titleVal = isTitle;
const subtitleVal = !isTitle;
return (
<Text title={titleVal} subtitle={subtitleVal}/>
)
}
}`,
errors: [{message: `'Typography.title' is deprecated. Please use 'Typography.heading' instead (fix is available).`}]
},
{
options: options,
code: `
import React, {Component} from 'react';
import {Typography, TextField} from 'our-source';
export default class OurList extends Component {
render() {
const titleVal = true;
return (
<TextField title={titleVal}/>
)
}
}`,
errors: [{message: `'Typography.title' is deprecated. Please use 'Typography.heading' instead (fix is available).`}]
},
{
options: options,
code: `
import React, {Component} from 'react';
import {Typography, TextField} from 'our-source';
export default class OurList extends Component {
render() {
const {isTitle} = this.props;
const titleVal = this.props.isTitle;
const subtitleVal = !this.props.isTitle;
return (
<TextField title={titleVal} subtitle={subtitleVal}/>
)
}
}`,
errors: [{message: `'Typography.title' is deprecated. Please use 'Typography.heading' instead (fix is available).`}]
},
{
options: options,
code: `
import React, {Component} from 'react';
import {Typography, TextField} from 'our-source';
export default class OurList extends Component {
render() {
const {isTitle} = this.props;
const titleVal = isTitle;
const subtitleVal = !isTitle;
return (
<TextField title={titleVal} subtitle={subtitleVal}/>
)
}
}`,
errors: [{message: `'Typography.title' is deprecated. Please use 'Typography.heading' instead (fix is available).`}]
},
// TODO: these tests are not currently supported, they might be supported when prop-value-shape-deprecation is merged (or we'll have to add support)
// {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we keeping these?
These are the use cases we don't support, right?
Maybe write a comment that explain these tests and that we can't/don't support them

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comments (TODOs)

// options: options,
// code: `
// import React, {Component} from 'react';
// import {Typography, Button} from 'our-source';
// export default class OurList extends Component {
// render() {
// const titleVal = true;
// return (
// <Button labelProps={{title: titleVal}}/>
// )
// }
// }`,
// errors: [{message: `'Typography.title' is deprecated. Please use 'Typography.heading' instead (fix is available).`}]
// },
// {
// options: options,
// code: `
// import React, {Component} from 'react';
// import {Typography, Button} from 'our-source';
// export default class OurList extends Component {
// render() {
// const titleVal = true;
// return (
// <Button title={titleVal}/>
// )
// }
// }`,
// errors: [{message: `'Typography.title' is deprecated. Please use 'Typography.heading' instead (fix is available).`}]
// },
// {
// options: options,
// code: `
// import React, {Component} from 'react';
// import {Typography, Card} from 'our-source';
// export default class OurList extends Component {
// render() {
// const titleVal = true;
// return (
// <Card.Section content={{title: titleVal}}/>
// )
// }
// }`,
// errors: [{message: `'Typography.title' is deprecated. Please use 'Typography.heading' instead (fix is available).`}]
// },
// {
// options: options,
// code: `
// import React, {Component} from 'react';
// import {Typography, TabBar} from 'our-source';
// export default class OurList extends Component {
// render() {
// const titleVal = true;
// return (
// <TabBar labelStyle={{title: titleVal}} selectedLabelStyle={{title: titleVal}}/>
// )
// }
// }`,
// errors: [{message: `'Typography.title' is deprecated. Please use 'Typography.heading' instead (fix is available).`}]
// },
// {
// options: options,
// code: `
// import React, {Component} from 'react';
// import {Typography, Label} from 'our-source';
// export default class OurList extends Component {
// render() {
// const titleVal = true;
// return (
// <Label labelProps={{title: titleVal}}/>
// )
// }
// }`,
// errors: [{message: `'Typography.title' is deprecated. Please use 'Typography.heading' instead (fix is available).`}]
// },
// TODO: this is not currently supported, it should be easily supported after the new utils are added
// {
// options: options,
// code: `
// import React, {Component} from 'react';
// import {Typography, Text as T} from 'our-source';
// export default class OurList extends Component {
// render() {
// const titleVal = true;
// return (
// <T title={titleVal}/>
// )
// }
// }`,
// errors: [{message: `'Typography.title' is deprecated. Please use 'Typography.heading' instead (fix is available).`}]
// },
],
});
6 changes: 6 additions & 0 deletions eslint-rules/tests/typography_deprecation.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,11 @@
"path": "Typography.deprecated",
"message": "Please use 'Typography.valid' instead (fix is available).",
"fix": "Typography.valid"
},
{
"path": "Typography.title",
"message": "Please use 'Typography.heading' instead (fix is available).",
"fix": "Typography.heading",
"components": ["Text", "TextField"]
}
]