Consistency references to useEffect, useRef etc.

[ad-m]

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 #18200 (comment)

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:

superset/superset-frontend/src/components/IndeterminateCheckbox/index.tsx

Line 85 in 076f774

React.useEffect(() => {

superset/superset-frontend/src/components/Icons/Icon.tsx

Line 20 in 99d15b2

import React, { useEffect, useRef, useState } from 'react';

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:

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.

{
  "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

[geido]

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.

[ad-m]

What about feature commited that remained as issues (as indicated in the morning meeting)? I am open to deliver this issue in a short time , but I am trying to discuss it in order to - as a fresh person in the project - adapt myself to the project, instead of the project to myself.

[ad-m]

The question that we have to answer is whether we forbid anything else to be referenced by "React".

It is worth taking a look at the imports:

$ find . -name '*.tsx' | grep -v 'node_modules' | xargs grep "from 'react'" | grep -v "import React from 'react';" | cut -d':' -f2- | sort -n | uniq -c | sort -n
      1 import React, { ChangeEvent, EventHandler } from 'react';
      1 import React, { ComponentType, ChangeEventHandler } from 'react';
      1 import React, { ComponentType, FunctionComponent, ReactElement } from 'react';
      1 import React, { createContext, useContext, useState } from 'react';
      1 import React, { createRef } from 'react';
      1 import React, { CSSProperties, Children, ReactElement } from 'react';
      1 import React, { CSSProperties, ComponentType, ReactNode } from 'react';
      1 import React, { CSSProperties, PureComponent, ReactNode } from 'react';
      1 import React, { CSSProperties, PureComponent } from 'react';
      1 import React, { CSSProperties, ReactNode, PureComponent } from 'react';
      1 import React, { CSSProperties, useCallback, useMemo } from 'react';
      1 import React, { CSSProperties, useEffect, useState, RefObject } from 'react';
      1 import React, { EventHandler, ChangeEvent, MouseEvent } from 'react';
      1 import React, { FC, ReactNode } from 'react';
      1 import React, { FC, useCallback, useMemo, useState } from 'react';
      1 import React, { FC, useCallback, useMemo } from 'react';
      1 import React, { FC, useCallback, useState } from 'react';
      1 import React, { FC, useEffect, useState } from 'react';
      1 import React, { FC, useMemo } from 'react';
      1 import React, { FC, useRef, useEffect, useState } from 'react';
      1 import React, { FormEvent } from 'react';
      1 import React, { FunctionComponentElement, useMemo } from 'react';
      1 import React, { FunctionComponent, useEffect, useState } from 'react';
      1 import React, { FunctionComponent, useState, useRef } from 'react';
      1 import React, { lazy, Suspense } from 'react';
      1 import React, { lazy } from 'react';
      1 import React, { LegacyRef } from 'react';
      1 import React, { MouseEventHandler, ReactNode } from 'react';
      1 import React, { PropsWithChildren, ReactNode } from 'react';
      1 import React, { PropsWithChildren } from 'react';
      1 import React, { ReactChild, useCallback } from 'react';
      1 import React, { ReactElement, useState } from 'react';
      1 import React, { ReactNode, CSSProperties } from 'react';
      1 import React, { ReactNode, ReactElement } from 'react';
      1 import React, { ReactNode, RefObject } from 'react';
      1 import React, { ReactNode, useCallback, useState } from 'react';
      1 import React, { ReactNode, useEffect, useState } from 'react';
      1 import React, { ReactNode, useState, useCallback } from 'react';
      1 import React, { ReactNode, useState, useEffect } from 'react';
      1 import React, { ReactNode, useState, useMemo, useEffect } from 'react';
      1 import React, { Suspense, useEffect } from 'react';
      1 import React, { SyntheticEvent, MutableRefObject, ComponentType } from 'react';
      1 import React, { useCallback, useEffect, useRef, useState } from 'react';
      1 import React, { useCallback, useEffect, useState, useMemo } from 'react';
      1 import React, { useCallback, useEffect } from 'react';
      1 import React, { useCallback, useMemo, useState } from 'react';
      1 import React, { useCallback, useRef } from 'react';
      1 import React, { useCallback, useState, useMemo, useEffect } from 'react';
      1 import React, { useContext, useEffect, useReducer } from 'react';
      1 import React, { useEffect, useMemo, useRef, useState } from 'react';
      1 import React, { useEffect, useMemo } from 'react';
      1 import React, { useEffect, useRef, useCallback } from 'react';
      1 import React, { useEffect, useState, FunctionComponent } from 'react';
      1 import React, { useEffect, useState, useCallback, useMemo } from 'react';
      1 import React, { useEffect, useState, useRef } from 'react';
      1 import React, { useMemo, CSSProperties } from 'react';
      1 import React, { useMemo, useState, useCallback, ReactElement } from 'react';
      1 import React, { useMemo, useState, useCallback, useEffect } from 'react';
      1 import React, { useMemo, useState, useEffect, useCallback } from 'react';
      1 import React, { useState, FunctionComponentElement, ChangeEvent } from 'react';
      1 import React, { useState, useCallback, useRef, FunctionComponent } from 'react';
      1 import React, { useState, useMemo, useCallback } from 'react';
      1 import React, { useState, useMemo, useEffect } from 'react';
      1 import React, { useState, useRef, useCallback } from 'react';
      1 import { ComponentType, useMemo } from 'react';
      2 import React, { CSSProperties, useMemo } from 'react';
      2 import React, { FC, useMemo, useState } from 'react';
      2 import React, { FunctionComponent, useState } from 'react';
      2 import React, { MouseEventHandler } from 'react';
      2 import React, { SVGProps } from 'react';
      2 import React, { useCallback, useEffect, useState } from 'react';
      2 import React, { useCallback, useMemo } from 'react';
      2 import React, { useCallback, useState } from 'react';
      2 import React, { useEffect, useRef, useState } from 'react';
      2 import React, { useEffect, useState, ReactNode } from 'react';
      2 import React, { useState, useEffect, useMemo } from 'react';
      3 import React, { FunctionComponent } from 'react';
      3 import React, { ReactElement } from 'react';
      3 import React, { useEffect, useMemo, useState } from 'react';
      3 import React, { useRef, useState } from 'react';
      3 import React, { useRef } from 'react';
      3 import React, { useState, ReactNode } from 'react';
      4 import React, { FunctionComponent, useState, useEffect } from 'react';
      4 import React, { useEffect } from 'react';
      4 import React, { useMemo, useState } from 'react';
      4 import React, { useMemo } from 'react';
      4 import React, { useState, useEffect } from 'react';
      4 import * as React from 'react';
      5 import React, { PureComponent } from 'react';
      5 import React, { RefObject } from 'react';
      5 import React, { useState, useMemo } from 'react';
      6 import React, { useCallback, useEffect, useMemo, useState } from 'react';
      7 import React, { CSSProperties } from 'react';
     10 import React, { FC } from 'react';
     10 import React, { useCallback } from 'react';
     10 import React, { useEffect, useState } from 'react';
     12 import React, { useState } from 'react';
     15 import React, { ReactNode } from 'react';
     19 } from 'react';

And references too:

$ find . -name '*.tsx' | grep -v 'node_modules' | xargs grep -E "React\.[A-Za-z]+?" -o | cut -d':' -f2- | sort -n | uniq -c | sort -n
      1 React.ComponentProps
      1 React.DetailedHTMLProps
      1 React.EventHandler
      1 React.FunctionComponent
      1 React.GetDerivedStateFromProps
      1 React.KeyboardEvent
      1 React.lazy
      1 React.MutableRefObject
      1 React.PropsWithoutRef
      1 React.RefCallback
      1 React.SVGProps
      1 React.SyntheticEvent
      1 React.useEffect
      1 React.useState
      1 React.WeakValidationMap
      2 React.createContext
      2 React.FocusEvent
      2 React.FormEvent
      2 React.ForwardRefExoticComponent
      2 React.HTMLAttributes
      2 React.Ref
      2 React.RefAttributes
      2 React.useRef
      3 React.ComponentClass
      3 React.Fragment
      4 React.Children
      4 React.createRef
      4 React.CSSProperties
      4 React.Key
      5 React.MouseEventHandler
      6 React.Component
      8 React.forwardRef
     10 React.cloneElement
     10 React.ReactElement
     10 React.RefObject
     12 React.MouseEvent
     13 React.memo
     18 React.ComponentType
     18 React.isValidElement
     23 React.ChangeEvent
     29 React.FC
     30 React.PureComponent
     60 React.ReactNode

It looks ReactNode (34+ imports) vs. React.ReactNode (60 references) and PureComponent (8+ imports) vs. React.PureComponent (30 references) is another common inconsistency.