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

Duplicate declaration "React" #31

Closed
novascreen opened this issue Nov 6, 2017 · 33 comments
Closed

Duplicate declaration "React" #31

novascreen opened this issue Nov 6, 2017 · 33 comments

Comments

@novascreen
Copy link

I'm using this plugin with next.js, which uses babel-plugin-react-require . It seems that the changes in 3bab74c are conflicting with that.
So now I get this:

Module build failed: TypeError: unknown: Duplicate declaration "React"
   5 | import _inherits from 'babel-runtime/helpers/inherits';
   6 | import React from 'react';
>  7 | import React from 'react';
     |        ^
@novascreen
Copy link
Author

I don't think this plugin should include this behavior, it's totally unexpected and duplicates what the above mentioned plugin does.

@ljharb
Copy link
Collaborator

ljharb commented Nov 6, 2017

It only duplicates what the above plugin does if you're using babel-plugin-react-require, which Airbnb isn't, for example.

Since this plugin actually checks to see if you have a React binding before adding the import, it seems like babel-plugin-react-require might have a bug by not checking that.

Specifically, https://github.com/vslinko/babel-plugin-react-require/blob/90f9994d936d85ad1b713afcfa256cb88d2b62e2/src/index.js#L14-L16 seems to suggest that it only adds the React binding when there is either no JSX, or a preexisting React binding - that logic should be changed to return early when there is a React binding present, regardless of whether jsx is present or not.

@mitchellhuang
Copy link

Also getting this error in Next.js after upgrading to 0.5.1

(function (exports, require, module, __filename, __dirname) { import React from "react";
                                                              ^^^^^^
SyntaxError: Unexpected token import
    at createScript (vm.js:56:10)
    at Object.runInThisContext (vm.js:97:10)
    at Module._compile (module.js:542:28)
    at Module._compile
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17)
    at require (internal/module.js:20:19)
(function (exports, require, module, __filename, __dirname) { import React from "react";
                                                              ^^^^^^

@ljharb
Copy link
Collaborator

ljharb commented Nov 7, 2017

@mitchellhuang what's your babel config that uses this plugin?

@mitchellhuang
Copy link

mitchellhuang commented Nov 7, 2017

@ljharb

.babelrc

{
  "plugins": [
    [
      "module-resolver", {
        "root": ["."],
        "alias": {
          "styles": "./styles"
        },
        "cwd": "babelrc"
    }],
    [
      "wrap-in-js",
      {
        "extensions": ["css$", "scss$"]
      }
    ],
    [
      "inline-react-svg"
    ]
  ],
  "presets": [
    [
      "next/babel",
      {
        "styled-jsx": {
          "plugins": [
            "styled-jsx-plugin-postcss"
          ]
        }
      }
    ]
  ],
  "ignore": []
}

next.config.js

const webpack = require('webpack');
const path = require('path');
const glob = require('glob');

module.exports = {
  webpack: (config) => {
    config.plugins.push(
      new webpack.EnvironmentPlugin([
        'NODE_ENV',
        'API_URI'
      ])
    );

    config.module.rules.push(
      {
        test: /\.(css|scss)/,
        loader: 'emit-file-loader',
        options: {
          name: 'dist/[path][name].[ext]'
        }
      },
      {
        test: /\.css$/,
        use: ['babel-loader', 'raw-loader', 'postcss-loader']
      },
      {
        test: /\.s(a|c)ss$/,
        use: ['babel-loader', 'raw-loader', 'postcss-loader',
          { loader: 'sass-loader',
            options: {
              includePaths: ['styles', 'node_modules']
                .map(d => path.join(__dirname, d))
                .map(g => glob.sync(g))
                .reduce((a, c) => a.concat(c), [])
            }
          }
        ]
      }
    );

    return config;
  },
  poweredByHeader: false
};

@ljharb
Copy link
Collaborator

ljharb commented Nov 7, 2017

What happens if you re-order your config so "presets" comes before "plugins"?

@JuanCaicedo
Copy link

@ljharb I'm also using next.js like @huangbong and I still get the same error if I move "presets" before "plugins" in my .babelrc

@JuanCaicedo
Copy link

JuanCaicedo commented Nov 7, 2017

I downgraded to v0.4.0 and this solved the problem. Not sure if the plugin is working well yet though

@ljharb
Copy link
Collaborator

ljharb commented Nov 7, 2017

The next/babel preset includes https://unpkg.com/next@4.1.4/dist/server/build/babel/preset.js which uses babel-plugin-react-require - which has a bug, as documented above.

This issue should be closed, and an issue filed on https://github.com/vslinko/babel-plugin-react-require instead.

@christophehurpeau
Copy link

christophehurpeau commented Mar 8, 2018

We have a file that imports svg without using react, so we didn't import react event if we don't use babel-plugin-react-require. Maybe a better course of action would be to check if multiple svg are imported and only add the import once?

@MrOpperman
Copy link

Ran into the same issue a few minutes ago, resolved by downgrading to 5.1.

You may need an appropriate loader to handle this file type.
|
| import React from 'react';
| import React from 'react';
| import React from 'react';

Was the kind of error I was receiving on all files that imported an SVG file. This issue happened once I migrated to webpack 4.

@catamphetamine
Copy link

@MrOpperman
I'm having the same on Webpack 4 with another babel plugin - babel-plugin-react-svg.
Maybe it's something Webpack-4 related.

@alecmev
Copy link

alecmev commented Apr 28, 2018

@ljharb

What happens if you re-order your config so "presets" comes before "plugins"?

The order doesn't matter, preset plugin visitors are always chained after user-supplied plugin visitors, at least until plugin ordering gets revamped (starting with babel/babel#5735).

This issue should be closed

It shouldn't. As mentioned by @christophehurpeau, this plugin adds a React import per each SVG, without any involvement from babel-plugin-react-require. Consider the following fixture:

import MySvg1 from './close.svg';
import MySvg2 from './close.svg';

export function MyFunctionIcon() {
  return MySvg1;
}

It results in the following output:

import React from 'react';
import React from 'react';

// ...

This could be fixed by having a hasReactBeenEnsured flag in Program.enter, but, IMO, this plugin shouldn't be adding a React import in the first place, for the same reasons Babel itself isn't adding a React import when it transforms JSX, as outlined in babel/babel#586 and babel/babel#2135 and babel/babel#2504.

@ljharb
Copy link
Collaborator

ljharb commented Apr 28, 2018

Thanks for clarifying - it’s absolutely a bug that it’s adding more than one react import when there’s more than one svg, and I’ll try to fix that ASAP before closing this.

Babel’s correct to not be opinionated; but this transform is for react, and assumes you’re not using it as a global since it’s 2018. Separately, babel/babel#2135 (comment) the (unresolved) reason why this is important.

Once the bug you’ve mentioned is fixed, the only remaining issue will be in the react-require transform, as detailed here.

@alecmev
Copy link

alecmev commented Apr 28, 2018

The logic in babel-plugin-react-require is correct, the bug/quirk is on Babel's side. Basically, the scope doesn't get updated when a plugin adds something to it, so path.scope.hasBinding('React') returns false even if another plugin has already added an import. More info here: babel/babel#6879

I have opened a PR in babel-plugin-react-require, which enables it to notice the addition of React imports by other plugins: vslinko/babel-plugin-react-require#18 But for that detection to work, this plugin must switch to path.unshiftContainer('body', newNode) instead of path.node.body.unshift(newNode).

The above will fix this particular bug, but not the situation where some plugin Foo does the same thing as babel-plugin-inline-react-svg, which would result in duplicate imports again. Potential solutions:

  1. Ship this as a preset, bundled with babel-plugin-react-require (I think the JSX detection will work as-is, as this plugin is using path methods directly).
  2. Integrate the functionality of babel-plugin-react-require (maybe by installing it as a dependency and passing through the visitor calls, but that's a bit dirty).
  3. Recommend people to install babel-plugin-react-require themselves, if needed (the best, IMO).

@ljharb
Copy link
Collaborator

ljharb commented Apr 28, 2018

@jeremejevs thank you for clarifying that it's a babel bug!

I think that the first steps are to 1) fix the "multiple SVG causes multiple imports" bug; and 2) use path.unshiftContainer('body', newNode) instead of path.node.body.unshift(newNode).

For the potential solutions you suggested: I agree that option 2 is a bit dirty, and option 1 won't work for airbnb, where for non-SVGs, we don't want React added silently. Option 3 adds an extra dependency for us that also won't work, for the same reason - because for non-SVGs, we demand explicit React imports and don't want to use a transform to add them.

@ljharb
Copy link
Collaborator

ljharb commented Apr 28, 2018

@jeremejevs so I'm trying to replicate the duplicate issue you're talking about, and it doesn't seem to be a problem. See #39.

@alecmev
Copy link

alecmev commented Apr 28, 2018

@ljharb It only happens when there's no pre-existing React import, which is why it has been surfacing only for those using babel-plugin-react-require, I presume.

@ljharb
Copy link
Collaborator

ljharb commented Apr 28, 2018

Got it, thanks. #39 now fixes that problem.

@alecmev
Copy link

alecmev commented Apr 28, 2018

Nice!

About the options - I guess the only one left then is to borrow the code from my PR and replace hasJSX stuff with hasSVGImport. But probably only after it's merged and tested, it's hard to predict problems in this plugin spaghetti.

@timneutkens
Copy link

timneutkens commented Oct 1, 2018

@ljharb I just published the PR to babel-plugin-react-require:
vslinko/babel-plugin-react-require#18

This issue can be closed now, as it'll be out on Next.js soon.

🙌

@jerlam06
Copy link

Issue still there...

@ljharb
Copy link
Collaborator

ljharb commented Dec 11, 2018

@jerlam06 please file an issue on babel-plugin-react-require

@mzaidse
Copy link

mzaidse commented Jan 17, 2019

Facing the same issue

| import React from "react";
> import React from "react";
| 

Using
"babel-plugin-inline-react-svg": "^0.4.0"
"next": "^7.0.2"

babelrc config

{
  "presets": [
    "next/babel"
  ],
  "plugins": [
    ["styled-components", { "ssr": true, "displayName": true, "preprocess": false } ],
    ["module-resolver", {
      "root": ["./"],
      "alias": {
        "lib": "./lib",
        "pages": "./pages",
        "components": "./components",
        "containers": "./containers",
        "store": "./store",
        "layouts": "./layouts",
        "static": "./static",
        "themes": "./themes",
        "utils": "./utils"
      }
    }],
    [
      "inline-react-svg",
      {
        "svgo": {
          "plugins": [
            {
              "removeAttrs": { "attrs": "(data-name)" }
            },
            {
              "cleanupIds": true
            }
          ]

        }
      }
    ],
    [
      "inline-import",
      {
        "extensions": [".css"]
      }
    ]
  ],
  "env": {
    "development": {
      "plugins": ["dotenv-import"]
    },
    "production": {
      "plugins": [
        ["dotenv-import", {
          "path": ".env.production"
        }]
      ]
    }
  }
}

kindly help me

@srosset81
Copy link

srosset81 commented Jan 20, 2019

It still doesn't work with this config:

"next": "^7.0.2",
"babel-plugin-react-require": "^3.0.1",
"babel-plugin-inline-react-svg": "^1.0.1"

However it seems the babel-plugin-react-require is not required by nextjs anymore. Removing it and importing the React package manually on each file solves the problem. :)

@pRdm
Copy link

pRdm commented Apr 18, 2019

@timneutkens this issue seems to be resurfacing with next@8.1.0, as babel-plugin-react-require@3.0.0 is listed as a dependency whereas the fix is in babel-plugin-react-require@3.0.1

see comments here: vercel/next.js#6281

not sure how to go about solving this apart from manually adding an import React statement?

@mhuggins
Copy link

I'm hitting this issue with next@8.1.0 as well.

@adamsoffer
Copy link

adamsoffer commented Aug 6, 2019

Yeah, I'm experiencing this issue as well on Next 9.03

@adamsoffer
Copy link

Can we reopen this issue?

@ljharb
Copy link
Collaborator

ljharb commented Aug 14, 2019

@adamsoffer it remains a bug with babel-plugin-react-require; this plugin already prevents a duplicate declaration.

@adamsoffer
Copy link

@ljharb gotcha thanks

@adamsoffer
Copy link

@ljharb apparently it was resolved in babel-plugin-react-require? vslinko/babel-plugin-react-require#17 (comment)

@ljharb
Copy link
Collaborator

ljharb commented Aug 14, 2019

If you're still having the problem, then I guess not :-)

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

No branches or pull requests