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

Identifier 'React' has already been declared with multiple external libraries #2987

Closed
derpoho opened this issue Apr 26, 2022 · 43 comments · Fixed by #3860
Closed

Identifier 'React' has already been declared with multiple external libraries #2987

derpoho opened this issue Apr 26, 2022 · 43 comments · Fixed by #3860

Comments

@derpoho
Copy link

derpoho commented Apr 26, 2022

What version of Remix are you using?

1.4.1

Steps to Reproduce

More Details:
React Version: 18
Yarn

  • Install remix for cloudflare pages setup
  • Add libraries like (react-spring, framer-motion, react-hook/mouse-postion)
  • Use them (simple import is enough)

Expected Behavior

Application/Website loads correctly when running wrangler pages dev ./public and remix watch

Actual Behavior

Website loads, shows for a few ms, then breaks. Console sends multiple errors:

Uncaught SyntaxError: Identifier 'React' has already been declared
Uncaught Error: Cannot initialize 'routeModules'. This normally occurs when you have server code in your client modules.
Hydration failed because the initial UI does not match what was rendered on the server.
react-dom.development.js:86 Warning: An error occurred during hydration. The server HTML was replaced with client content in <#document>.

The first error shows the error into the installed packages from above, always the same line (the first react import)
var React = __toESM(require_react());

This happens to a lot of modules. You can fix that by renaming React to something else within the package and patching those with npx patch-package. But this makes things hard as you have to patch a lot of packages. It looks like something is going wrong during compilation (which naturally is part of remix - non-configurable).

Can somebody reproduce this issue?

Edit:
Node Version: v16.13.2
Mac M1 Max

@kiliman
Copy link
Collaborator

kiliman commented Apr 26, 2022

I've had this issue as well. I believe it's an issue with esbuild bundler. If you minify the code, it seems to work.

evanw/esbuild#475 (comment)

@mcansh mcansh added the external label May 3, 2022
@nickjs
Copy link

nickjs commented May 12, 2022

I can also reproduce this issue with threejs and the @react-three suite of packages. I can prepare a very simple repro repo if helpful.

@agcty
Copy link

agcty commented May 30, 2022

I can also also reproduce this issue with threejs and react-three-fiber as well.

@agcty
Copy link

agcty commented May 30, 2022

It seems like esbuild is the issue in this case (evanw/esbuild#475). Is there an option for remix to pass config options to esbuild without needing to patch packages? @kiliman

@agcty
Copy link

agcty commented Jun 1, 2022

Update:

You can definitely reproduce it with https://github.com/pmndrs/drei (no matter if it's cloudflare pages, node, etc.)

I believe this issue comes up if a library doesn't minify, or at least concatenate its output into a single file and therefore has multiple React imports.

Here's how drei's output looks like:
image
image

Whereas @react-three/fiber works and if you look into its output, you'll see that all modules are bundled into a single cjs and esm file respectively.

image

Same case with framer motion (orks)

image

I am not sure how compiling works under the hood in remix but I believe that in production builds, it minifies everything, potentially stripping out multiple react imports and therefore it works, while in dev builds it doesn't do that. That would explain @kiliman's observation that if you minify the code, it seems to work.

If this is really the case, I think that this is actually a remix/esbuild bug that needs to be addressed rather than trying to restructure the build output of libraries.

@jaschaio
Copy link

jaschaio commented Jun 14, 2022

Here is an incredible ugly workaround, based on a comment in the esbuild issue:

Modify your package.json:

  "scripts": {
-   "build": "remix build",
+   "build": "NODE_OPTIONS='-r ./esbuild-overwrite' remix build",
-   "dev": "remix dev",
+   "dev": "NODE_OPTIONS='-r ./esbuild-overwrite' remix dev"
  },

Create a file esbuild-overwrite.js with the Following contents:

const esbuild = require( 'esbuild' );
const Module = require( 'module' );

const originalRequire = Module.prototype.require;
const originalBuild = esbuild.build;

const reactShim = [
    ...Object.keys( require( 'react' ) ).map( k => `export const ${ k } = window.React.${ k }` ),
    'const React = window.React; export default React;',
].join( '\n' );
const reactDomShim = Object.keys( require( 'react-dom' ) ).map( k => `export const ${ k } = window.ReactDOM.${ k }` ).join( '\n' );

// https://esbuild.github.io/plugins/#using-plugins
const externaliseReactPlugin = {
    name: 'react',
    setup( build ) {
        
        build.onResolve( { filter: /^react$/ }, ( args ) => ( {
            path: args.path,
            namespace: 'react-ns',
        } ) );

        build.onLoad( { filter: /.*/, namespace: 'react-ns' }, ( args ) => ( {
            contents: reactShim,
            loader: 'js',
        } ) );
        
        build.onResolve( { filter: /^react-dom$/ }, ( args ) => ( {
            path: args.path,
            namespace: 'react-dom-ns',
        } ) );

        build.onLoad( { filter: /.*/, namespace: 'react-dom-ns' }, ( args ) => ( {
            contents: reactDomShim,
            loader: 'js',
        } ) );

    }
};

function build( options ) {

    if ( options.platform !== 'browser' )
        return originalBuild( options );

    return originalBuild( {
        ...options,
        external: [
            ...( options.external ) ? options.external : [],
            'react',
            'react-dom',
        ],
        plugins: [
            ...( options.plugins ) ? options.plugins : [],
            externaliseReactPlugin,
        ],
    } );

};

Module.prototype.require = function ( id ) {

    // when remix requires esbuild, it will get our modified build function from above
    if ( id === 'esbuild' )
        return { ...esbuild, build };

    return originalRequire.apply( this, arguments );

};

Add react and react-dom as global objects to your window, for example via script tags within your root.jsx file:

export default App() {

    return (return (
        <html lang="en">
            <head>
                <Meta />
                <Links />
+               <script crossorigin src="https://unpkg.com/react@18/umd/react.production.min.js"></script>
+               <script crossorigin src="https://unpkg.com/react-dom@18/umd/react-dom.production.min.js"></script>/head>
            <body>
                <Outlet />
                <ScrollRestoration />
                <Scripts />
                <LiveReload />
            </body>
        </html>
    );

}

@kiliman
Copy link
Collaborator

kiliman commented Jun 14, 2022

Interesting. I think we need to determine if this is indeed an esbuild issue or not, and if so, what can be done to fix the root cause.

But yeah, anything that unblocks you sounds good to me.

@agcty
Copy link

agcty commented Jun 15, 2022

Interesting. I think we need to determine if this is indeed an esbuild issue or not, and if so, what can be done to fix the root cause.

But yeah, anything that unblocks you sounds good to me.

I agree, we definitively need to be able to say this is either a remix, esbuild or library issue.

Been trying to bundle libraries in a different way and see when this issue occurs, will let you know when I've got something more solid.

@kiliman
Copy link
Collaborator

kiliman commented Jun 15, 2022

Has anyone tried using esbuild with these libraries outside of Remix to see if the issue occurs? That would be the true test. If it works there, then it may be how Remix is splitting client/server bundles that's breaking this.

@jaschaio
Copy link

In my case I am not using any third party library but just an internal component library (that has it's own package.json). If I don't use remix and just esbuild it works. I only get the problem when using remix + esbuild

@kiliman
Copy link
Collaborator

kiliman commented Jun 15, 2022

Ok, can you provide a simple repo of you component library? Something that will reproduce the error. Then we can figure out what's causing the issue. Thanks!

@jaschaio
Copy link

I did in this issue: #3102

For reference:

Steps to Reproduce

Here is a sample repository with the below steps: https://github.com/jaschaio/remix-bug

Create a new directory

mkdir monorepo
cd monorepo

Create a new package directory within it

mkdir packages
cd packages

Within the packages directory create a new remix project

npx create-remix@latest

Choose "Just the basics" and "Express Server" (this doesn't matter) and install the dependencies.

Create another directory within the /packages directory

mkdir ../controls
cd ../controls

Run npm init, name the package @local/controls and add react and react-dom as peerDependencies into the package.json

{
  "name": "@local/controls",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "type": "module",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
+  "peerDependencies": {
+    "react": "^17.0.2",
+    "react-dom": "^17.0.2"
+  }
}

Create a React Component, for example Text.js within the /packages/controls directory. The important thing is that you use some hook like useCallback:

import React, { useCallback } from 'react';

const Text = ( { handleChange, value } ) => {

    const onChange = useCallback( ( event ) => handleChange( event.target.value ), [ handleChange ] );

    return (
        <input
            type="text"
            onChange={ onChange }
            value={ value }
        />
    );

};

export default Text;

Go back to the /packages/website directory and install the other local package via:

npm i ../controls

Add it as a dependency to the serverDependenciesToBundle within the /packages/website/remix.config.js:

/**
 * @type {import('@remix-run/dev').AppConfig}
 */
module.exports = {
  ignoredRouteFiles: ["**/.*"],
+  serverDependenciesToBundle: [
+    /@local\/controls/,
+  ],
  // appDirectory: "app",
  // assetsBuildDirectory: "public/build",
  // serverBuildPath: "build/index.js",
  // publicPath: "/build/",
};

Within the /packages/website/app/routes/index.jsx directory, import the component, remove the default content and place the Component within it:

import { useState } from 'react';
import Text from '../components/Text';

export default function Index() {

    const [ value, setValue ] = useState( '' );

    return (
        <div>
            <Text value={ value } handleChange={ setValue } />
        </div>
    );
}

Run npm run dev from within the /packages/website directory

When visiting localhost:3000 you will get the Following error:

Uncaught Error: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.

@agcty
Copy link

agcty commented Jun 15, 2022

Honestly, this issue is so incredibly strange. Upgraded to 1.6.0 and it persists under certain circumstances.
Here's a repro: https://github.com/agcty/remix-three-test/tree/remix-16 (branch: remix-16, steps to reproduce in readme)

For some strange reason, if you have 2 routes with the affected library, it works, or you can have one route but then you need to apply a patch to remix so it also minifies dev builds.

@agcty
Copy link

agcty commented Jun 15, 2022

Ok, I've been diving fairly deep into the remix compiler and I've been trying to replicate the issues isolated with esbuild, and think I was finally able to achieve a similar output.

What I found is that when certain libraries are used and specific edge cases apply (e.g e.g only one file that imports an affected package), it somehow clashes with the react shim that is automatically injected resulting in something like:

// public/build/routes/index-IG57A5WQ.ts

// line 1-9
import {
  React,
  __commonJS,
  __export,
  __toESM,
  _extends,
  init_react,
  require_react
} from "/build/_shared/chunk-E36X77XE.js";

// line 43356
var React = __toESM(require_react());

I am actually not convinced anymore that the issue is due to evanw/esbuild#475. That issue is related to useless imports, not var redeclarations. Granted, it would be better for esbuild to keep imports to a minimum but it should not affect correctness (evanw/esbuild#475 (comment)).

I am now more of the opinion, that this might be a remix compiler configuration issue. For this reason, I also think the "external" tag of this issue should be removed.

Obviously, we need the react shim one way or another but I'm fairly certain there is a way to make it compatible, but no idea yet.

I have created a reproduction repo here: https://github.com/agcty/remix-three-test/tree/remix-16

And here is a 2 min video that walks through the repro: https://www.youtube.com/watch?v=X-T1OrHxEqg

@kiliman @chaance @mcansh

@kiliman
Copy link
Collaborator

kiliman commented Jun 15, 2022

Awesome. Thanks for the deep dive. I'll take a look when I get a chance. I know this is an annoying issue for many developers.

@MichaelDeBoey
Copy link
Member

MichaelDeBoey commented Jun 28, 2022

This problem is probably fixed by #3588 & can be tested by v1.6.2-pre.0

@jacob-ebey
Copy link
Member

This problem is probably fixed by #3588 & can be tested by v1.6.2-pre.0

This doesn't fix it unfortunately. That was the minifyIdentifiers flag, not the minifySyntax in that PR.

@agcty
Copy link

agcty commented Jul 2, 2022

This problem is probably fixed by #3588 & can be tested by v1.6.2-pre.0

This doesn't fix it unfortunately. That was the minifyIdentifiers flag, not the minifySyntax in that PR.

So does this mean a fix isn't possible for now or this is a "won't fix" type of issue?

Edit: I remembered wrong on the react-three-fiber issue, that is the minifyIdentifiers flag that fixes the duplicate React identifiers. minifyIdentifiers isn't something we want to enable in dev right now. (#3588)

@chaance
Copy link
Collaborator

chaance commented Jul 5, 2022

So does this mean a fix isn't possible for now or this is a "won't fix" type of issue?

Definitely looking to fix this. It's still at the top of my priority list.

@jsellam
Copy link

jsellam commented Jul 5, 2022

Thank you, I have the same issue with web3 https://www.npmjs.com/package/web3 on client side (used in useCallback).
I have this message : Error: Cannot initialize 'routeModules'. This normally occurs when you have server code in your client modules.

@timothymalcham
Copy link

timothymalcham commented Jul 15, 2022

I'm also experiencing this same error (Identifier 'React' has already... and Cannot initialize 'routeModules') when trying to use https://www.npmjs.com/package/@grapecity/activereports-react on the client side only.

@remix-run remix-run deleted a comment from joshuaellis Jul 28, 2022
@mcansh mcansh linked a pull request Jul 28, 2022 that will close this issue
2 tasks
@chaance
Copy link
Collaborator

chaance commented Jul 28, 2022

This won't make it in time for the next release, but initial testing shows the fix should land when #3860 is merged.

@mcansh
Copy link
Collaborator

mcansh commented Aug 1, 2022

This won't make it in time for the next release, but initial testing shows the fix should land when #3860 is merged.

if anyone here wants to take 0.0.0-experimental-ea38b6458 for a spin, that'd be great!

@lswest
Copy link
Contributor

lswest commented Aug 1, 2022

This won't make it in time for the next release, but initial testing shows the fix should land when #3860 is merged.

if anyone here wants to take 0.0.0-experimental-ea38b6458 for a spin, that'd be great!

I ran into this issue today with two packages' internal dependencies: use-onclickoutside (internal dependency use-latest), and @reach/dialog (internal dependency react-focus-lock). Somehow these two packages were yielding an additional React definition.

Results

Installing the above version allowed the pages to work properly again!

Extra information

The below is just extra information for anyone wanting to debug/investigate this further, you can safely ignore this otherwise.

I figured out the packages by following the console.log error regarding duplicate definitions (one linked to the root bundle, one to another chunk). Then based on the comment in the file with the path to the file, I figured out which packages they were and which dependencies required them via npm list. Leaving this information here for anyone who needs it.

Side note regarding the issue appearing: The odd thing is I didn't touch either of those dependencies since the last time the code was working (last Friday, the 29th of July) and today. In remix 1.4.2 only one page wasn't working (related to @reach/dialog, upon updating to 1.6.7 no pages were working (the issue appeared in use-onclickoutside, which I use in the site header's popover navigation). Presumably the esbuild version was updated between those versions?

@mcansh
Copy link
Collaborator

mcansh commented Aug 1, 2022

before the update coming in #3860, esbuild has been at 0.14.22 since february 17th (50e428b) which was done in 1.2.2

@lswest
Copy link
Contributor

lswest commented Aug 1, 2022

Thanks for the info. No idea why the problem waited until today to appear then or what else I might have done to trigger it, but the main thing is the experimental build fixes it 😄

@jaschaio
Copy link

jaschaio commented Aug 2, 2022

This won't make it in time for the next release, but initial testing shows the fix should land when #3860 is merged.

if anyone here wants to take 0.0.0-experimental-ea38b6458 for a spin, that'd be great!

Looks like it works, thanks a lot!

Any idea which release this will hit?

@kokotom
Copy link

kokotom commented Aug 3, 2022

I have just tried 0.0.0-experimental-ea38b6458 and still not working for me :(

@mcansh
Copy link
Collaborator

mcansh commented Aug 3, 2022

@kokotom same error message? any particular dependencies you've narrowed down?

@kokotom
Copy link

kokotom commented Aug 3, 2022

@kokotom same error message? any particular dependencies you've narrowed down?

yes same error message.
for me the library was https://www.npmjs.com/package/@syncfusion/ej2-gantt

@lswest
Copy link
Contributor

lswest commented Aug 3, 2022

@kokotom - just to check (because I made this mistake the first time too) - did you change to the exact version from @mcansh's post? I had ^0.0.0-experimental-ea38b6458 in my package.json, so npm install grabbed a different version that it felt was "newer". Make sure it's just the version number (0.0.0-experimental-ea38b6458) in package.json.

@mcansh
Copy link
Collaborator

mcansh commented Aug 3, 2022

@lswest @kokotom yeah i just tried the latest experimental and all is fine with that library :)

demo
commit

@kokotom
Copy link

kokotom commented Aug 4, 2022

@lswest @kokotom yeah i just tried the latest experimental and all is fine with that library :)

demo commit

Oh man terribly sorry.
The package was:
@syncfusion/ej2-react-gantt
Glad you noticed it yourself :)

However I noticed that I didn't pin the version as mentioned by @lswest.
After doing so I'm getting a different error:

Warning: React.jsx: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.

It happened both with react 17.0.2 and 18.2.0

@saranshgrover
Copy link

experimental-ea38b6458 solved it for me. Any ideas when this would be pushed onto a proper release?

@mcansh
Copy link
Collaborator

mcansh commented Aug 8, 2022

experimental-ea38b6458 solved it for me. Any ideas when this would be pushed onto a proper release?

finishing up some windows testing updates before i merge the PR just to be safe, but it should go out this week unless there's something i missed in the esbuild changelog that would prevent that.

edit: we're gonna let it simmer a bit longer, but i'll cut a new experimental based on our latest release when we cut that

@Swapnull
Copy link

Swapnull commented Aug 11, 2022

@lswest @kokotom yeah i just tried the latest experimental and all is fine with that library :)
demo commit

Oh man terribly sorry. The package was: @syncfusion/ej2-react-gantt Glad you noticed it yourself :)

However I noticed that I didn't pin the version as mentioned by @lswest. After doing so I'm getting a different error:

Warning: React.jsx: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.

It happened both with react 17.0.2 and 18.2.0

I have just tried 0.0.0-experimental-ea38b6458 after having these same problems and get this same error.
image
My entry.server.tsx is exactly the same as the ones from the examples. I am on react 17.0.2 and macOS 12.0.1

The library I am having issues with is @100mslive/react-sdk

@raminrez
Copy link

In my case problem was importing the hole react module in one of the components
import * as React from 'react'

@patrikholcak
Copy link

patrikholcak commented Aug 15, 2022

@raminrez You shouldn’t be forced to rename a variable just because it occurs somewhere else in the bundle (perhaps even a library you didn’t write.) What you’re seeing is a prime example of this bug, not a solution!

All the imports in a module should be isolated to just that module. It’s the compiler’s job to figure out the best way to rename/merge the import statements when concatenating the modules into one bundle.

@chaance
Copy link
Collaborator

chaance commented Aug 24, 2022

Looks like we were finally able to get this resolved! Updated to v1.7.0-pre.0 here and everything runs swimmingly. We should be able to close this after the stable release is published 🥳

@MichaelDeBoey
Copy link
Member

Closed by #3860

@Swapnull
Copy link

@lswest @kokotom yeah i just tried the latest experimental and all is fine with that library :)
demo commit

Oh man terribly sorry. The package was: @syncfusion/ej2-react-gantt Glad you noticed it yourself :)
However I noticed that I didn't pin the version as mentioned by @lswest. After doing so I'm getting a different error:
Warning: React.jsx: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.
It happened both with react 17.0.2 and 18.2.0

I have just tried 0.0.0-experimental-ea38b6458 after having these same problems and get this same error. image My entry.server.tsx is exactly the same as the ones from the examples. I am on react 17.0.2 and macOS 12.0.1

The library I am having issues with is @100mslive/react-sdk

@chaance FYI updating to remix 1.7.0 now gives me this error and completely breaks my app in the same way I mentioned here (which I had working perfectly fine by using patch-package to remove some react imports from modules).

@chaance
Copy link
Collaborator

chaance commented Aug 26, 2022

@Swapnull mind opening a new issue with a repro for this one?

@Swapnull
Copy link

@chaance Sure, for posterity, new issue opened #4080 with repro

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

Successfully merging a pull request may close this issue.