You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/02/08 02:08:27 UTC
[GitHub] [superset] ad-m opened a new issue #18607: Consistency references to useEffect, useRef etc.
ad-m opened a new issue #18607:
URL: https://github.com/apache/superset/issues/18607
**Is your feature request related to a problem? Please describe.**
I would like to ask about the governance process for linting rules.
I received the comment:
> Can we import the `useEffect` from above consistently with other parts of the application?
_Originally posted by @geido in https://github.com/apache/superset/pull/18200#discussion_r800929136_
Linting rules are often a sore point or at least a subjective feeling. Are there general rules for managing eslint rules in a project that I should follow when proposing changes?
We do not import that consistently in project, for example:
https://github.com/apache/superset/blob/076f774daf3c0bad41b6f162e96f21949ddee1f7/superset-frontend/src/components/IndeterminateCheckbox/index.tsx#L85
https://github.com/apache/superset/blob/99d15b25b0177e0b62c25fe859a1f31262b0f7e8/superset-frontend/src/components/Icons/Icon.tsx#L20
I would like to propose that the above-mentioned suggestion was presented as a linting rule and enforced.
**Describe the solution you'd like**
I have developed a suitable linter:
```javascript
export default function (context) {
return {
// This listener will be called for all expression like React.useRef, React.useEffect
'MemberExpression[object.name="React"][property.name=/use.*|forward/]': function (node) {
return context.report({
node,
message: `Import ${node.property.name} directly instead of referencing it via ${node.object.name}.${node.property.name}`
});
}
};
}
```
You can check it at https://astexplorer.net/ .
**Describe alternatives you've considered**
Alternative `no-restricted-syntax` (see
https://eslint.org/docs/developer-guide/selectors ) can be configured to avoid import, but it will reduce the precision of the error message.
```json
{
"rules": {
"no-restricted-syntax": ["error", "MemberExpression[object.name='React'][property.name=/use.*|forward/]"]
}
}
```
We might also do not introduce any linting rules, but it will increase a maintainer burden to communicate & validate that rules manually.
**Additional context**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on issue #18607: Consistency references to useEffect, useRef etc.
Posted by GitBox <gi...@apache.org>.
geido commented on issue #18607:
URL: https://github.com/apache/superset/issues/18607#issuecomment-1033667257
Hello @ad-m I think this is a good idea! Thanks for sharing. I am moving this issue to discussions as we are looking to use the Github issues for true bugs only.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org