Dynamic import() fails in next 5.xx

lkostrowski commented 6 years ago

I have been using this piece of code to asynchronously load one of heavy component on my client side. It was working like a charm:

const DynamicChart = dynamic( import('../components/LineChart/LineChart'), { loading: () => <SectionPlaceholder><Loader /></SectionPlaceholder>, ssr: false, }, );

Since update to v5.0.0 I get webpack error, but only when I open the parent page from frontend route. When I directly load from server, it get's loaded. When I get it once (from first load) and go back to this page on frontend, it works too.

This is the error I get:

I tried import(...).then(...).catch(...) but none of chains were triggered.

The problem is that I can't reproduce it always.

I'm also using next-routes if that matters

I assume that problem occurs because of undefined part in webpack. Any ideas how to debug the cause?

My package dependencies:

 "dependencies": {
    "babel-plugin-styled-components": "^1.5.0",
    "dotenv": "^5.0.0",
    "dotenv-webpack": "^1.5.4",
    "enzyme": "^3.3.0",
    "eslint-plugin-jest": "^21.7.0",
    "express": "^4.16.2",
    "glamor": "^2.20.40",
    "glamorous": "^4.11.4",
    "htmlescape": "^1.1.1",
    "isomorphic-fetch": "^2.2.1",
    "lodash": "^4.17.5",
    "next": "^5.0.0",
    "next-redux-wrapper": "^1.3.5",
    "next-routes": "^1.3.0",
    "polished": "^1.9.2",
    "react": "^16.2.0",
    "react-dom": "^16.2.0",
    "react-extras": "^0.5.0",
    "react-icons": "^2.2.7",
    "react-markdown": "^3.2.0",
    "react-modal": "^3.1.12",
    "react-no-ssr": "^1.1.0",
    "react-redux": "^5.0.6",
    "react-sizeme": "^2.3.6",
    "recharts": "^1.0.0-beta.10",
    "redux": "^3.7.2",
    "redux-logger": "^3.0.6",
    "redux-thunk": "^2.2.0",
    "reselect": "^3.0.1",
    "socket.io": "^2.0.4",
    "socket.io-client": "^2.0.4",
    "yargs": "^11.0.0"
  },
  "devDependencies": {
    "@storybook/addon-actions": "^3.3.12",
    "@storybook/addon-info": "^3.3.12",
    "@storybook/addon-links": "^3.3.12",
    "@storybook/addons": "^3.3.12",
    "@storybook/react": "^3.3.12",
    "@types/enzyme": "^3.1.8",
    "@types/jest": "^22.1.1",
    "@types/next": "^2.4.7",
    "@types/node": "^9.4.1",
    "@types/react": "^16.0.36",
    "@types/recharts": "^1.0.13",
    "@types/reselect": "^2.2.0",
    "@types/socket.io": "^1.4.31",
    "@types/storybook__react": "^3.0.6",
    "babel-core": "^6.26.0",
    "babel-eslint": "^7.2.3",
    "babel-plugin-recharts": "^1.1.1",
    "babel-plugin-transform-decorators-legacy": "^1.3.4",
    "babel-preset-env": "^1.6.1",
    "babel-preset-es2015": "^6.24.1",
    "babel-preset-stage-0": "^6.24.1",
    "babili-webpack-plugin": "^0.1.2",
    "console-probe": "^2.0.4",
    "create-component-app": "^1.6.2",
    "eslint": "^4.17.0",
    "eslint-config-airbnb": "^16.1.0",
    "eslint-plugin-babel": "^4.1.2",
    "eslint-plugin-import": "^2.8.0",
    "eslint-plugin-jsx-a11y": "^6.0.3",
    "eslint-plugin-react": "^7.6.1",
    "eslint-plugin-styled-components-config": "0.0.2",
    "jest": "^22.1.4",
    "prop-types": "^15.6.0",
    "redux-devtools-extension": "^2.13.2",
    "webpack-bundle-analyzer": "^2.10.0"
  }

[x] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

Component gets loaded every time

Current Behavior

Sometimes component is not loaded and webpack tries to load something with undefined in name

aputinski commented 6 years ago

I've created reproducible test case https://github.com/aputinski/next-dynamic-import-test-case

rauchg commented 6 years ago

Thanks for the nice reproducible test case @aputinski. We'll take a look!

arunoda commented 6 years ago

@aputinski @lkostrowski Thanks for the detailed error report and for the sample repo. Yes. I can reproduce the error.

We got this due to the the importing module is located inside the pages directory. As a quick fix, try to move those modules outside of pages directory and import.

Like this:

import React from 'react';

export default class extends React.Component {
  static async getInitialProps() {
    const post = await import('../lib/post.json')
    return {
      post
    }
  }
  render () {
    return <h1>{this.props.post.title}</h1>
  }
}

lkostrowski commented 6 years ago

Hi guys, thanks for input.

Unfortunately there must be at lease one more reason than @aputinski posted. Let me paste my simplified page:

const DynamicChart = dynamic(
    import('../components/LineChart/LineChart')
        .then((c) => {
            console.log('Component loaded');
            return c;
        }).catch(err => console.log(err)),
    {
        loading: () => <SectionPlaceholder><Loader /></SectionPlaceholder>,
        ssr: false,
    },
);

class Dashboard extends React.Component {
    static async getInitialProps() {
        return {};
    }

    render() {
        return (
            <UserArea>
                <Grid>
                    <DashboardSection>
                        <DynamicChart />
                    </DashboardSection>
                    <DashboardSection>
                        <DynamicChart />
                    </DashboardSection>
                </Grid>
            </UserArea>
        );
    }
}

So 1: I create dynamic in module scope 2: I load component from different directory

As I said before I have problem to reproduce it all the times

I will do some checks today: 1: I will try to wrap this page with another component (I would have done it anyway but I'm working on prototype) 2: I will check f.e if moving dynamic() factory to getInitialProps() helps

lkostrowski commented 6 years ago

Ok I have new input. Changing structure to more nested seems to make no difference.

However I detected how to accurately reproduce problem. It happens only after first server start (dev or prod), so it's enough to force server to build page again and chunks are built successfully. Even if I refresh page on another view and go to page with dynamic components, they are build well.

Isn't it connected to server caching? I don't know what's happening under the hood, but looks like page is server before chunks created?

Can I provide any more information?

aputinski commented 6 years ago

@arunoda Moving the file still produces the same error, but as @lkostrowski said, it only happens upon first starting the server. Refreshing the page or starting on that page results in no error.

aputinski commented 6 years ago

I've also tried giving the imported file a hard-coded chunk name using /* webpackChunkName */

aputinski commented 6 years ago

@arunoda If you can give me a few pointers on where to start, I can take a stab at debugging the issue.

gerhardsletten commented 6 years ago

I am only experience this in dev-mode, and here its happening on each new route in the app:

Start with npm run dev, and visit localhost:3000 (ok)
Navigate to /route-b (fails with error above)
Hard refresh /route-b (ok)
Navigate to /route-c (fails with error above)
Hard refresh /route-c (ok)

Each route in my app has a dynamic require for the component used. Get the same error in the examples with-dynamic-import by initially visit localhost:3000/about and the click the Home-link. But as said, only in dev-mode.

lkostrowski commented 6 years ago

@gerhardsletten good that you mentioned that - just check and I also see this error on dev server uff

exogen commented 6 years ago

I'm currently experiencing this on version 5.0.0, does anyone know if it's fixed in newer versions or have more recent info?

PaulRBerg commented 6 years ago

Playing around with 6.0.0-canary.3 and I can confirm it's still the same.

dihmeetree commented 6 years ago

@rauchg @arunoda any updates on this? Just ran into this issue myself :/

dihmeetree commented 6 years ago

I'd also like to point out.. this bug only occurs during development.. the production build works fine with loading chunks.

zhengqingxin commented 6 years ago

Anyone has a temporary solution ?

zhengqingxin commented 6 years ago

I use With No SSR in development mode as a temporary solution

const isDev = process.env.NODE_ENV !== 'production'
const PCSearch = dynamic(import('page/pc/index/Search'),{ ssr: !isDev })

dcalhoun commented 6 years ago

@arunoda I created another test case where I'm experiencing this same error. You're proposed changes (i.e. moving the file out of the pages directory) did not help. Is there any other guidance for resolving this? Thanks!

brad-decker commented 6 years ago

@arunoda having this issue on Next 6, exact same behavior with the exception of ssr: false does not help. If i hit the route with a dynamic import directly all is fine. If i hit it as a routing even within the app it throws errors for every dynamic import:

screen shot 2018-05-03 at 2 40 33 pm

brad-decker commented 6 years ago

this issue is being reported in several places with different descriptions. #4175 #4059 etc.

If i run next build before next i don't hit the same issue as before. Its like the dev server is trying to hit files in the main build folder instead of the ephemeral files created in dev.

vjpr commented 6 years ago

Also be aware that errors are silenced in dynamic components with {ssr: false}. See https://github.com/zeit/next.js/pull/3009.

I just encountered the error myself, and I have patched next to fix the silent error problem I mentioned above, so this is a separate issue.

stramel commented 6 years ago

I am also running into this error getting Error: Loading chunk 6 failed. This was my import statement with dynamic.

const Map = dynamic(import('../../components/Map'), {
  ssr: false,
})

I swapped it out for a raw import statement and still had the issue, I don't believe this is related to the dynamic that Next.js offers but more of something with the webpack config.

class A extends React.Component {
  constructor(props) {
    super(props)
    this.state = { module: null }
  }
  componentDidMount() {
    import('../../components/Map').then(module => this.setState({ module: module.default }))
  }
  render() {
    const { router } = this.props
    const { module: Map } = this.state

    return (
      <div>
        {Map && <Map />}
      </div>
    )
  }
}

NOTE: I'm using 6.0.3-canary.1

stramel commented 6 years ago

I'm curious if there is a way instead of trying to parse the import ourselves, if we could just pass off as much processing as possible to the import to webpack's Parser? That would probably allow dynamic imports with Computed module specifiers.

eli0shin commented 6 years ago

After a couple hours of banging my head against the wall, I found that by using the multi-component example for import() from the docs and exporting the wrapper, the error goes away.

Not the prettiest solution, but the api functions as expected and there are no more errors

jesstelford commented 6 years ago

@eli0shin do you have a concrete code example of before vs after for that fix?

eli0shin commented 6 years ago

I realized that I still get errors occasionally in development, before it wasn't even working in production. It is worth noting that loaded-component is not capable of being loaded on the server and had to be loaded in asynchronously inside on a component that skips the server render.

import React, { Component } from 'react';
import dynamic from 'next/dynamic';

const Loader = dynamic({
  modules: () => {
    const LoadedComponent = import('loaded-component');
    return { LoadedComponent };
  },
  render: (props, { LoadedComponent }) => (
    <Wrapper
      LoadedComponent={LoadedComponent}
      {...props}
    />
  ),
  ssr: false,
});

const Wrappper Wysiwyg  = ({LoadedComponent}) =>{
    const { LoadedComponent } = this.props;
    return (
      <LoadedComponent />
    );
  };

export default Loader;

jesstelford commented 6 years ago

That's a shame, thanks for sharing the partial workaround though! 👍

arnaud-brun commented 6 years ago

Hi ! I encounter the related issue when working with react-pdf which cannot be rendered outside the client. So my first thought was to use dynamic.

I think I solved the problem with getInitialProps function. Here is a snippet :

class PdfViewer extends React.PureComponent {
  constructor(props) {
    super(props);
  }

  render() {
    if (this.props.isServer)
      return (<Loader />)

    return (
      <div>
        <div className="document">
          <Document
            file={data}
            rotate={rotate}
            onLoadSuccess={this.onDocumentLoad}
            >
            <Page pageNumber={pageNumber} scale={scale}/>
          </Document>
        </div>
     )
  }
}

PdfViewer.getInitialProps = async function(context) {
  const isServer = typeof window === 'undefined'
  return { isServer }
}

I hope this will help ! Still looking for a proper solution with dynamic feature

michaeljonathanblack commented 6 years ago

I am also experiencing this issue. It's important to note that it didn't occur in Next 3, and possibly Next 4. Some regression was introduced since then.

zvictor commented 6 years ago

This bug seems to have been underrated. I would like to emphasise that such bug makes the dynamic imports feature unreliable and therefore useless for serious projects. Thus, I would suggest reestimating it's importance for this project.

If it is not possible to foresee a fix for it in the short term, which is totally fine and understandable, we should then hide it in the docs to avoid more frustration and more issues being reported from new users trying it out.

@timneutkens would you be the right person to take a look at that?

imagine10255 commented 6 years ago

I have also encountered this problem, find this issue version: nextjs 5.1.0

vjpr commented 6 years ago

build/static/commons/manifest.js is recompiled many times while the app runs.

I think this is the issue, these files should be static (unchanging) shouldn't they?

I also notice that main.js is compiled twice...if you enable [hash] you can see two separate files are created.

It is configured by the CommonsChunkPlugin here: server/build/webpack.js:302.

The issue appears to be a race condition whereby a stale manifest.js is retrieved from the server.

There is a line in the manifest.js file that contains all the available chunks:

/******/        script.src = __webpack_require__.p + "" + ({"0":"main.js","1":"bundles/pages/_document.js","2":"bundles/pages/_app.js","3":"bundles/pages/_error.js","5":"bundles/pages/analytics.js","6":"bundles/pages/overview.js","7":"chunks/_quantitec_intranav_map_0b0839261db7b60c6ddf2824cfd4e835","8":"bundles/pages/map.js"}[chunkId]||chunkId) + "-" + {"0":"a03f371cba25b2a7a353","1":"78550462ac237623af14","2":"ce770dc184e8107d9392","3":"a8b57bdb879379391237","5":"783eb7ab48d1525905aa","6":"3a46e0123295a0cdeb4a","7":"5deeabe4c6c6987dbbb5","8":"a0bedc5798c3e304bf3c"}[chunkId] + ".js";

The reason undefined shows up is because the chunk's hash is not included in the manifest.js file.

You should find that reloading the page used cmd+r makes the dynamic component show up.

If you look at build/static/commons/manifest.js before and then just after you try to load the dynamic component, you will see that your chunk appears there, but a stale version is used by the client.

@timneutkens @ptomasroos Would be great if you could shed light on how this is suppose to work.

What does the comment // We use a manifest file in development to speed up HMR mean?

I noticed that a chunkhash is appended to it in production...seeing as it is being recompiled constantly in development, maybe we need the chunkhas in dev too.

Also, why are there two CommonsChunkPlugin?

Also,

              dev && !isServer && new _webpack.default.optimize.CommonsChunkPlugin({
                name: 'manifest.js',
                filename: dev ? 'static/commons/manifest.js' : 'static/commons/manifest-[chunkhash].js'
              })].filter(Boolean)

dev ? X : Y is unnecessary because the code path only executed if dev is true.

Also, if you manually set dev to false, you get the error:

Cannot use [chunkhash] for chunk in 'static/commons/manifest-[chunkhash].js' (use [hash] instead)

timneutkens commented 6 years ago

This issue will be fixed with webpack 4.

vjpr commented 6 years ago

@timneutkens When do you think this will be released in next? Weeks or months?

Just wondering whether its worth me trying to find a fix for it.

I am trying to disable the manifest.js by passing Infinity to minChunks, but it still uses it. It seems that __webpack_require__.e which is require.ensure as used in next/dynamic, is be called inside the manifest.js commons chunk for some reason.

Okay so removing manifest.js commons chunk still causes error. Its a race condition as I outlined earlier in the thread.

Disabling all commons chunks might fix it. Or finding a way to prevent the Commons chunk from churning between SSR webpack builds.

I think the issue might be that because we are compiling server-side on each page navigation, this means the commons chunks are recompiled too, and the patch/on-demand chunks sent to the client are referencing the new commons chunks, that should be the same until the browser is refreshed.

Tried disabling commons chunks but new issues arose.

mjanssen commented 6 years ago

Dynamically importing modules in componentDidMount using System.import('module'); seems to work while developing. However, when running a production build, the file seems not to be loaded correctly. When using the dynamic import from next/dynamic it doesn't seems to be functional during developing.

vjpr commented 6 years ago

There is a workaround here: https://github.com/zeit/next.js/issues/4642#issuecomment-399151228

mjanssen commented 6 years ago

The workaround is not functional (or I'm implementing it wrongly).

We've got:

> lib/common.js

const dynamics = () => [import('flickity')];

> Module using flickity
import '../../lib/common'

class A extends React.Component {
  async componentDidMount() {
    const Flickity = await System.import('flickity');
  }
  ...
}

But the reference to the Flickity module still breaks.

Note. The reason we're importing Flickity dynamically is because right now they instantly rely on window, which is obviously not available during SSR.

timneutkens commented 6 years ago

The reason I'm saying it'll be solved when webpack 4 lands, is that I'm reworking the whole next/dynamic implementation so that it does nothing "special" and will just use default webpack import() which it doesn't currently. This will allow for setting the chunk name comment etc. And also solve this issue.

Regarding a timeline, we'll release it whenever it's ready. I'm currently working on this.

vjpr commented 6 years ago

@mjanssen I just tried it out and it works.

Try using:

import dynamic from 'next/dynamic'

import './lib'

const Flickity = dynamic(import('flickity'), {ssr: false}))

The idea with the workaround is to make sure that import('flickity') is in all your pages so that when loading any SSR page, the manifest.js will contain references to all dynamic chunks it will need in the future.

You can add the dynamic component imports to pages/_document.js.

stramel commented 6 years ago

I moved the import './lib' to my _app.js and it seems to be working

mjanssen commented 6 years ago

All right, so your fix works @vjpr, thanks. However, I've used the 'forbidden fruit' from React, to pass a ref={} to a component and call functions in the mounted component. The functions within the component are not passed over when dynamic importing the component.

So:

> Slider.js

class Slider extends React.Component {
  next = () => {
    // Slide to next slide
  };

  prev = () => {
    // Slide to next slide
  };

  render() {
    ...
  }
}

> ClassUsingSlider.js

import dynamic from 'next/dynamic';

const Slider = dynamic(import('./Slider'), {ssr: false});

class ClassUsingSlider extends React.Component {
  onClick = () => {
    this.slider.next();
  };

  render() {
    return <Slider ref={c => (this.slider = c)} />;
  }
}

vercel / next.js

Dynamic import() fails in next 5.xx #3775

Expected Behavior

Current Behavior