Skip to content
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

update-react-imports adds extra import * from React #283

Open
anark opened this issue Oct 26, 2020 · 10 comments · May be fixed by #305
Open

update-react-imports adds extra import * from React #283

anark opened this issue Oct 26, 2020 · 10 comments · May be fixed by #305

Comments

@anark
Copy link

anark commented Oct 26, 2020

When an import is defined as

import React, { useState } from 'react'

(usually due to automatically added imports)

The update-react-imports codemod transforms it to

import { useState } from 'react';
import * as React from 'react';

Instead of just

import { useState } from 'react';
@m-rutter
Copy link

Yeah, I've notice it does this in typescript projects if you use types like React.ReactNode or React.ComponentType.

@kelly-tock
Copy link

same here

@monic-shixi
Copy link

+1
it will trigger import/no-duplicates lint.

@yurkimus
Copy link

yurkimus commented Feb 6, 2021

Any updates here? The tool does same thing with imports...

Also... How I can clean React imports easily, if codemod can't do this?

@milesj
Copy link

milesj commented Mar 17, 2021

Bumping this.

@lexanth
Copy link

lexanth commented Apr 7, 2021

For me at least, this was in a typescript project when using React as a namespace for types. The issue in the codemod is that it isn't handling TSQualifiedName the same as QualifiedTypeIdentifier.

I've modded the codemod to do what I needed (someone could probably make a PR of this)

/**
 * (c) Facebook, Inc. and its affiliates. Confidential and proprietary.
 *
 * @format
 */

module.exports = function (file, api, options) {
  const j = api.jscodeshift
  const printOptions = options.printOptions || {}
  const root = j(file.source)
  const destructureNamespaceImports = options.destructureNamespaceImports

  // https://github.com/facebook/jscodeshift/blob/master/recipes/retain-first-comment.md
  function getFirstNode() {
    return root.find(j.Program).get('body', 0).node
  }

  // Save the comments attached to the first node
  const firstNode = getFirstNode()
  const { comments } = firstNode

  function isVariableDeclared(variable) {
    return (
      root
        .find(j.Identifier, {
          name: variable,
        })
        .filter(
          (path) =>
            path.parent.value.type !== 'MemberExpression' &&
            path.parent.value.type !== 'QualifiedTypeIdentifier' &&
            // Added this
            path.parent.value.type !== 'TSQualifiedName' &&
            path.parent.value.type !== 'JSXMemberExpression',
        )
        .size() > 0
    )
  }

  // Get all paths that import from React
  const reactImportPaths = root
    .find(j.ImportDeclaration, {
      type: 'ImportDeclaration',
    })
    .filter((path) => {
      return (
        (path.value.source.type === 'Literal' ||
          path.value.source.type === 'StringLiteral') &&
        (path.value.source.value === 'React' ||
          path.value.source.value === 'react')
      )
    })

  // get all namespace/default React imports
  const reactPaths = reactImportPaths.filter((path) => {
    return (
      path.value.specifiers.length > 0 &&
      path.value.importKind === 'value' &&
      path.value.specifiers.some(
        (specifier) => specifier.local.name === 'React',
      )
    )
  })

  if (reactPaths.size() > 1) {
    throw Error(
      'There should only be one React import. Please remove the duplicate import and try again.',
    )
  }

  if (reactPaths.size() === 0) {
    return null
  }

  const reactPath = reactPaths.paths()[0]
  // Reuse the node so that we can preserve quoting style.
  const reactLiteral = reactPath.value.source

  const isDefaultImport = reactPath.value.specifiers.some(
    (specifier) =>
      specifier.type === 'ImportDefaultSpecifier' &&
      specifier.local.name === 'React',
  )

  // Check to see if we should keep the React import
  const isReactImportUsed =
    root
      .find(j.Identifier, {
        name: 'React',
      })
      .filter((path) => {
        return path.parent.parent.value.type !== 'ImportDeclaration'
      })
      .size() > 0

  // local: imported
  const reactIdentifiers = {}
  const reactTypeIdentifiers = {}
  let canDestructureReactVariable = false
  if (isReactImportUsed && (isDefaultImport || destructureNamespaceImports)) {
    // Checks to see if the react variable is used itself (rather than used to access its properties)
    canDestructureReactVariable =
      root
        .find(j.Identifier, {
          name: 'React',
        })
        .filter((path) => {
          return path.parent.parent.value.type !== 'ImportDeclaration'
        })
        .filter(
          (path) =>
            !(
              path.parent.value.type === 'MemberExpression' &&
              path.parent.value.object.name === 'React'
            ) &&
            !(
              path.parent.value.type === 'QualifiedTypeIdentifier' &&
              path.parent.value.qualification.name === 'React'
            ) &&
            !(
              // Added this
              (
                path.parent.value.type === 'TSQualifiedName' &&
                path.parent.value.left.name === 'React'
              )
            ) &&
            !(
              path.parent.value.type === 'JSXMemberExpression' &&
              path.parent.value.object.name === 'React'
            ),
        )
        .size() === 0

    if (canDestructureReactVariable) {
      // Add React identifiers to separate object so we can destructure the imports
      // later if we can. If a type variable that we are trying to import has already
      // been declared, do not try to destructure imports
      // (ex. Element is declared and we are using React.Element)
      root
        .find(j.QualifiedTypeIdentifier, {
          qualification: {
            type: 'Identifier',
            name: 'React',
          },
        })
        .forEach((path) => {
          const id = path.value.id.name
          if (path.parent.parent.value.type === 'TypeofTypeAnnotation') {
            // This is a typeof import so it isn't actually a type
            reactIdentifiers[id] = id

            if (reactTypeIdentifiers[id]) {
              canDestructureReactVariable = false
            }
          } else {
            reactTypeIdentifiers[id] = id

            if (reactIdentifiers[id]) {
              canDestructureReactVariable = false
            }
          }

          if (isVariableDeclared(id)) {
            canDestructureReactVariable = false
          }
        })

      // Added this
      root
        .find(j.TSQualifiedName, {
          left: {
            type: 'Identifier',
            name: 'React',
          },
        })
        .forEach((path) => {
          const id = path.value.right.name
          reactIdentifiers[id] = id
          // We don't tend to use type imports
          // Comment line above out and uncomment this to use type imports
          // Also ignoring typeof imports?
          // reactTypeIdentifiers[id] = id

          // if (reactIdentifiers[id]) {
          //   canDestructureReactVariable = false
          // }

          if (isVariableDeclared(id)) {
            canDestructureReactVariable = false
          }
        })

      // Add React identifiers to separate object so we can destructure the imports
      // later if we can. If a variable that we are trying to import has already
      // been declared, do not try to destructure imports
      // (ex. createElement is declared and we are using React.createElement)
      root
        .find(j.MemberExpression, {
          object: {
            type: 'Identifier',
            name: 'React',
          },
        })
        .forEach((path) => {
          const property = path.value.property.name
          reactIdentifiers[property] = property

          if (isVariableDeclared(property) || reactTypeIdentifiers[property]) {
            canDestructureReactVariable = false
          }
        })

      // Add React identifiers to separate object so we can destructure the imports
      // later if we can. If a JSX variable that we are trying to import has already
      // been declared, do not try to destructure imports
      // (ex. Fragment is declared and we are using React.Fragment)
      root
        .find(j.JSXMemberExpression, {
          object: {
            type: 'JSXIdentifier',
            name: 'React',
          },
        })
        .forEach((path) => {
          const property = path.value.property.name
          reactIdentifiers[property] = property

          if (isVariableDeclared(property) || reactTypeIdentifiers[property]) {
            canDestructureReactVariable = false
          }
        })
    }
  }

  if (canDestructureReactVariable) {
    // replace react identifiers
    root
      .find(j.QualifiedTypeIdentifier, {
        qualification: {
          type: 'Identifier',
          name: 'React',
        },
      })
      .forEach((path) => {
        const id = path.value.id.name

        j(path).replaceWith(j.identifier(id))
      })

    // Added this
    root
      .find(j.TSQualifiedName, {
        left: {
          type: 'Identifier',
          name: 'React',
        },
      })
      .forEach((path) => {
        const id = path.value.right.name

        j(path).replaceWith(j.identifier(id))
      })

    root
      .find(j.MemberExpression, {
        object: {
          type: 'Identifier',
          name: 'React',
        },
      })
      .forEach((path) => {
        const property = path.value.property.name

        j(path).replaceWith(j.identifier(property))
      })

    root
      .find(j.JSXMemberExpression, {
        object: {
          type: 'JSXIdentifier',
          name: 'React',
        },
      })
      .forEach((path) => {
        const property = path.value.property.name

        j(path).replaceWith(j.jsxIdentifier(property))
      })

    // Add exisiting React imports to map
    reactImportPaths.forEach((path) => {
      const specifiers = path.value.specifiers
      for (let i = 0; i < specifiers.length; i++) {
        const specifier = specifiers[i]
        // get all type and regular imports that are imported
        // from React
        if (specifier.type === 'ImportSpecifier') {
          if (
            path.value.importKind === 'type' ||
            specifier.importKind === 'type'
          ) {
            reactTypeIdentifiers[specifier.local.name] = specifier.imported.name
          } else {
            reactIdentifiers[specifier.local.name] = specifier.imported.name
          }
        }
      }
    })

    const regularImports = []
    Object.keys(reactIdentifiers).forEach((local) => {
      const imported = reactIdentifiers[local]
      regularImports.push(
        j.importSpecifier(j.identifier(imported), j.identifier(local)),
      )
    })

    const typeImports = []
    Object.keys(reactTypeIdentifiers).forEach((local) => {
      const imported = reactTypeIdentifiers[local]
      typeImports.push(
        j.importSpecifier(j.identifier(imported), j.identifier(local)),
      )
    })

    if (regularImports.length > 0) {
      j(reactPath).insertAfter(
        j.importDeclaration(regularImports, reactLiteral),
      )
    }
    if (typeImports.length > 0) {
      j(reactPath).insertAfter(
        j.importDeclaration(typeImports, reactLiteral, 'type'),
      )
    }

    // remove all old react imports
    reactImportPaths.forEach((path) => {
      // This is for import type React from 'react' which shouldn't
      // be removed
      if (
        path.value.specifiers.some(
          (specifier) =>
            specifier.type === 'ImportDefaultSpecifier' &&
            specifier.local.name === 'React' &&
            (specifier.importKind === 'type' ||
              path.value.importKind === 'type'),
        )
      ) {
        j(path).insertAfter(
          j.importDeclaration(
            [j.importDefaultSpecifier(j.identifier('React'))],
            reactLiteral,
            'type',
          ),
        )
      }
      j(path).remove()
    })
  } else {
    // Remove the import because it's not being used
    // If we should keep the React import, just convert
    // default imports to named imports
    let isImportRemoved = false
    const specifiers = reactPath.value.specifiers
    for (let i = 0; i < specifiers.length; i++) {
      const specifier = specifiers[i]
      if (specifier.type === 'ImportNamespaceSpecifier') {
        if (!isReactImportUsed) {
          isImportRemoved = true
          j(reactPath).remove()
        }
      } else if (specifier.type === 'ImportDefaultSpecifier') {
        if (isReactImportUsed) {
          j(reactPath).insertAfter(
            j.importDeclaration(
              [j.importNamespaceSpecifier(j.identifier('React'))],
              reactLiteral,
            ),
          )
        }

        if (specifiers.length > 1) {
          const typeImports = []
          const regularImports = []
          for (let x = 0; x < specifiers.length; x++) {
            if (specifiers[x].type !== 'ImportDefaultSpecifier') {
              if (specifiers[x].importKind === 'type') {
                typeImports.push(specifiers[x])
              } else {
                regularImports.push(specifiers[x])
              }
            }
          }
          if (regularImports.length > 0) {
            j(reactPath).insertAfter(
              j.importDeclaration(regularImports, reactLiteral),
            )
          }
          if (typeImports.length > 0) {
            j(reactPath).insertAfter(
              j.importDeclaration(typeImports, reactLiteral, 'type'),
            )
          }
        }

        isImportRemoved = true
        j(reactPath).remove()
      }
    }

    if (!isImportRemoved) {
      return null
    }
  }

  // If the first node has been modified or deleted, reattach the comments
  const firstNode2 = getFirstNode()
  if (firstNode2 !== firstNode) {
    firstNode2.comments = comments
  }

  return root.toSource(printOptions)
}

Usage:

npx jscodeshift --verbose=2 --ignore-pattern="**/node_modules/**" --parser ts --extensions=tsx,ts,jsx,js --transform ./path/to/the/transform.js --destructureNamespaceImports=true ./path/to/src/

@hieuhoangtrung
Copy link

thanks @lexanth for your code. Just needs to update the usage to specify the param --parser=tsx. But there are some cases have been skipped with my codebase, may need to look deeper to improve your code

@oriooctopus
Copy link

oriooctopus commented Oct 12, 2021

From what I can tell, this is intentional. React is deprecating the import React from 'react' style import in the future, and import * from 'react' is the correct way to go. Additionally, that style of import has to be on it's own line.
Eslint's no-duplicate-import does not trigger an error on eslint 7.28.0

The tests in this PR also show that this was intentional

@grumd
Copy link

grumd commented Jul 19, 2022

I simply replaced import * as React from 'react' with import type * as React from 'react' across my entire codebase after running the codemod. Worked for me. There was only a dozen or so files that needed to be manually fixed after that.

@sparrowek
Copy link

I have thought that it should remove unnecessary import React from 'react' but now it converts it to import * as React from 'react';

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.