reactjs / react-codemod

React codemod scripts
MIT License
4.2k stars 288 forks source link

Consider providing an option for update-react-transform to convert namespace imports #277

Closed billyjanitsch closed 3 years ago

billyjanitsch commented 4 years ago

Currently, the update-react-transform transform assumes that if a file is currently using a namespace React import, then the developer wants it to stay that way. I don't think that's generally, or even usually, true.

Issues with importing React as a pseudo-default import have been known for a long time, so many codebases categorically avoid importing React this way. This leaves two options:

import * as React from 'react'

function MyComponent() {
  const [count, setCount] = React.useState(0)
  return <div>{count}</div>
}

or:

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

function MyComponent() {
  const [count, setCount] = useState(0)
  return <div>{count}</div>
}

The downside of the first option is having to write, e.g., React.useState at every callsite of a React import. The downside of the second option is having two React imports in most files.

So there's not a clear winner, but some codebases have settled on the first option as "least bad" since multiple imports are relatively uncommon. Many of those codebases would really prefer to be using named imports; they just can't because React needs to be in scope for JSX and shouldn't be imported as a default.

Now the JSX scope requirement is lifted, those codebases can migrate to use only named imports! This is great, but it's unfortunate that the codemod doesn't do this automatically, especially because the code was written in this way specifically in preparation for this JSX transform update.

I understand why you might not want this as the default behavior, but would you consider providing an option for the codemod to replace namespace imports with named ones?

/cc @lunaruan @gaearon

billyjanitsch commented 4 years ago

As a workaround, one can run a find/replace on the codebase for import * as React from 'react' -> import React from 'react' immediately before running the codemod (since it converts default imports to named imports).

lunaruan commented 4 years ago

Hey! We originally decided not to convert any named imports (ex. import * as React from 'react') because it was pointed out that some people might want to keep the import * as React from 'react' pattern. However, you make a good point about how there are codebases that would prefer to use destructured named imports (ex. import {useState} from 'react'). I'll add an option to allow this. Thanks for reporting!

billyjanitsch commented 4 years ago

Yeah, I totally understand why some codebases may not want this, but an option would be fantastic. Thanks @lunaruan, and thanks for providing the codemod to begin with! 🙂

billyjanitsch commented 3 years ago

Hi @lunaruan, thank you for adding the option! Is it possible to publish a new version of the package so that it can be used? Thanks!

gaearon commented 3 years ago

released