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

Dynamic import fails on named exports #2940

Closed
1 task done
jstcki opened this issue Sep 11, 2017 · 7 comments · Fixed by #4639
Closed
1 task done

Dynamic import fails on named exports #2940

jstcki opened this issue Sep 11, 2017 · 7 comments · Fixed by #4639

Comments

@jstcki
Copy link
Contributor

jstcki commented Sep 11, 2017

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

Expected Behavior

Dynamically importing a named export should work like this:

const DynamicFoo = dynamic(import('../components/Foo').then(m => m.Foo));

Additionally, default exports need also to be explicitly imported when following the spec – something that TypeScript needs you to do, otherwise you'll get an error.

// Accessing m.default explicitly is mandatory when using TypeScript
const DynamicFoo = dynamic(import('../components/Foo').then(m => m.default));

Current Behavior

When accessing a page with an import like above, I get the following error:

Path must be a string. Received undefined
TypeError: Path must be a string. Received undefined
    at assertPath (path.js:28:11)
    at join (path.js:1239:7)
    at loadChunks (/xyz/node_modules/next/dist/server/render.js:528:37)

(oddly, this doesn't happen when the named import is added to a hot-reloaded page)

Context

@jstcki
Copy link
Contributor Author

jstcki commented Sep 11, 2017

The problem seems to be at https://github.com/zeit/next.js/blob/master/server/build/babel/plugins/handle-import.js#L17, where __webpackChunkName is set on the required module, which doesn't get passed on through .then(m => m.Foo).

An ugly workaround to this problem:

const DynamicFoo = dynamic(import('../components/Foo').then(m => {
  const {Foo} = m;
  Foo.__webpackChunkName = m.__webpackChunkName;
  return Foo;
}));

Could a better solution be to set __webpackChunkName on the promise itself (but also feels hacky)?

@jstcki
Copy link
Contributor Author

jstcki commented Sep 29, 2017

Also, FWIW, the semantics of next/dynamic are off: dynamic(import(...)) doesn’t imply lazy loading because it looks like import() is called immediately.

dynamic(() => import(...)) would be much more in line with how one would expect the code to work from looking at it. C.f. https://github.com/thejameskyle/react-loadable

P.S.: Less important but as a bonus, dynamic() could just return a regular next.js-agnostic lazy-loading component when not being babel-transformed (which would make it possible to e.g. include it in a style guide outside of next.js).

@timneutkens
Copy link
Member

@arunoda

@jstcki
Copy link
Contributor Author

jstcki commented Nov 13, 2017

Another idea: maybe import.meta could be a suitable vehicle for transporting/getting the webpack chunk name? I'm not sure if that's even possible or supported by webpack …

Anyway, I'd be motivated to investigate more and help out with this, but only if there's interest on your part.

@ichyr
Copy link

ichyr commented Feb 7, 2018

@herrstucki @timneutkens Was this issue solved in some way? I'm on next@4.2.3 and getting the same errors as specified in the description of this issue.

@arunoda
Copy link
Contributor

arunoda commented Feb 7, 2018

@ichyr it's not resolved yet. You need to still do this: #2940 (comment)

We did all these changes to allow seamless SSR for dynamic imports.
When the React new async components became stable, we'll move to webpack's original import implementation.

@vjpr
Copy link

vjpr commented May 7, 2018

Errors in dynamic imports are silenced which could confound this issue. See #3897.

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

Successfully merging a pull request may close this issue.

5 participants