Fix Typography lint catching non truthy props

[M-i-k-e-l]

Description

Fix Typography lint catching non truthy props

Changelog

Fix Typography lint catching non truthy props

[ethanshar]

I assume this check is because people use typography modifiers like this?

<Text bodyBold />

And the prop doesn't have an actual value (it behaves as boolean true) so you assume it's a modifier?

If that's right I think you might be missing something.
What if someone renders this

const useBodyBold = <some condition>;
<Text bodyBold={useBodyBold} />

In this case the value will exist and can be true or false.

Maybe the check should be more explicit and ignore boolean values

[M-i-k-e-l]

Yes, it was the title case (which can be an actual title in some components).
That's a good point, I'll add a test and fix accordingly.

[M-i-k-e-l]

From what I can tell there's only the Identifier type (no boolean, string etc), so we can't tell what the value is unless we start to dig in deeper into the tree, I don't think it's worth it at this stage, WDYT?

For reference I'll leave two example node.value:
title (string):
{"type":"JSXExpressionContainer","start":155,"end":162,"loc":{"start":{"line":5,"column":27},"end":{"line":5,"column":34}},"expression":{"type":"Identifier","start":156,"end":161,"loc":{"start":{"line":5,"column":28},"end":{"line":5,"column":33},"identifierName":"title"},"name":"title","range":{"0":156,"1":161},"_babelType":"Identifier"},"range":{"0":155,"1":162},"_babelType":"JSXExpressionContainer"}
isItDeprecated (true):
{"type":"JSXExpressionContainer","start":103,"end":119,"loc":{"start":{"line":3,"column":29},"end":{"line":3,"column":45}},"expression":{"type":"Identifier","start":104,"end":118,"loc":{"start":{"line":3,"column":30},"end":{"line":3,"column":44},"identifierName":"isItDeprecated"},"name":"isItDeprecated","range":{"0":104,"1":118},"_babelType":"Identifier"},"range":{"0":103,"1":119},"_babelType":"JSXExpressionContainer"}

[ethanshar]

I checked in https://astexplorer.net
And inside JSXExpressionContainer you can find an expression that holds the value.
You can check typeof value to get the type.

[M-i-k-e-l]

Nice site, thanks :)
Not sure how I can get the value though (again, without traversal).
Do you have a way to do that without traversal?
If not, I suggest we open a task and do that after the refactor.

Code examples:
1.
import React, {Component} from 'react';
import {Typography} from 'our-source';
import {List} from 'another-source';
export default class OurList extends Component {
render() {
const title = 'bla';
return (
<List.Item title={title}/>
)
}
}

2. Same but with:
   const title = true;

[M-i-k-e-l]

It'll help because generally components that have Props that extend TextProps or TypographyModifiers do not have the title prop.
So, if they have title as a prop, it'll be the typography, and we don't need to check for a value.
I think that if a component will have two props with the same name (i.e. a prop and a modifier that have the same name), it'll be a bigger problem than the lint rules, so it should not happen.

[ethanshar]

Ok sounds good..
But we can still have this, no?

const useTitle = ....;
<Text title={useTitle}>My Text</Text>

[M-i-k-e-l]

Yes, but if the user has title it's already "bad", i.e. deprecated, no matter what the value is

[ethanshar]

ohhhh right right.
ok sounds good.
Let's do that - check on specific components

[M-i-k-e-l]

Done.
I've added some commented tests for stuff we don't yet support (renamed components - should be easily supported after the refactor, props of components - might be supported with Lidor's PR).

[ethanshar]

Looks good!
just couple of comments..

[ethanshar]

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

[M-i-k-e-l]

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

[ethanshar]

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

[M-i-k-e-l]

Added comments (TODOs)