Next.js: Redundant React Imports in next.js/examples

[awareness481]

Feature request

Is your feature request related to a problem? Please describe.

Currently most examples in the examples folder include react imports in files under pages which is not needed. I can't recall if it ever was needed in Next.js (Perhaps some examples with an older next.js version need the imports)

Describe the solution you'd like

Remove react imports from files that don't need. Method of testing is up to you to decide but I presume running the project, checking for console errors and making sure the functionality is the same should be sufficient?

Alternatives Considering whether the changes are worth it

Is this something that the Next.js team think should be changed? It's perfectly reasonable if you guys think this isn't worth the effort. With that said, I would love the honor of contributing to the Next.js repo, even with something that is probably considered a chore.

I don't know if there are any technical differences with redundantly importing React but it could be argued that's it's bad practice.

Additional context

Some components are written with classes where as far as I can tell React imports are needed.

import React, { PureComponent } from 'react'
import { connect } from 'react-redux'
import Link from 'next/link'
import { startClock, serverRenderClock } from '../actions'
import Examples from '../components/examples'

class Index extends PureComponent {
//...
}

Should those be updated to functional components or be left alone?

I also forked the repo just to showcase what the differences would be using the with-redux and with-redux-thunk examples. with-redux-thunk uses a class component in pages/index.js so there I didn't remove the React import.

Git diff

[timneutkens]

Next.js automatically adds the React import when JSX is used indeed. However keep in mind that we do still need to import React from 'react' when the React variable is used. Besides that yeah it seems like a good one to contribute 👍 Marked as good first issue.

[mattcarlotta]

My fault for the redundant imports in that example above. I've submitted a PR to remove them from the with-redux-thunk example. That said, there's definitely some missing documentation about React being automatically imported within JSX files.

On that note, since it's automatically imported, would it be more efficient to provide React as a global in development (that way, default React exports, like React.Component, don't need to be reimported)? Understandably, it still wouldn't make sense to provide the named exports nor to have React be a global in production.

[jmgr2996]

Hello @timneutkens, is it ok to send a PR for each example? I'm working on a list with those examples that use a redundant React import

Maybe we could list them in this issue and mark the solved ones

[timneutkens]

My fault for the redundant imports in that example above. I've submitted a PR to remove them from the with-redux-thunk example. That said, there's definitely some missing documentation about React being automatically imported within JSX files.

On that note, since it's automatically imported, would it be more efficient to provide React as a global in development (that way, default React exports, like React.Component, don't need to be reimported)? Understandably, it still wouldn't make sense to provide the named exports nor to have React be a global in production.

We've thought about it but it makes optimizing harder later on, e.g. when esmodules support is in a high percentage of browsers.

[jmgr2996]

Is it possible to avoid the implicit import if there's a React instance or maybe warn the user when import the namespace? Has this situation any impact on the size of the bundle?

[willianjusten]

@mvfsillva and I were removing in some examples, we pushed on #13169 and he's going to take the rest after =)

[awareness481]

Assuming the last PR will be merged, the only remaining examples are:

• with-linaria
• with-overmind

For with-overmind I'm waiting to see whether PR https://github.com/zeit/next.js/pull/13385 will be accepted before I remove the react imports.

with-linaria $ npm run dev failed on my system but I didn't have time to investigate.

[awareness481]

@timneutkens With PR https://github.com/zeit/next.js/pull/13422 the only remaining example is with-linaria, excluding class components.

I was unable to run with-linaria on my system so I opened an issue https://github.com/zeit/next.js/issues/13423 . I'll investigate further if nobody picks up the issue within a week.

P.S I felt it was relevant to tag you, please feel free to say if it bothered you.

[Timer]

Exciting work! Thanks to all involved.
FWIW, we get all notifications, so it's no biggy!

[nullhook]

Next.js automatically adds the React import when JSX is used indeed. However keep in mind that we do still need to import React from 'react' when the React variable is used. Besides that yeah it seems like a good one to contribute 👍 Marked as good first issue.

Is this still valid? In the latest version of nextjs I didn't import React but I still was able to use React.useState() without any issues.

[timneutkens]

@nullhook you should always import hooks as it's non-standard that React is available.