gregberge / loadable-components

The recommended Code Splitting library for React ✂️✨
https://loadable-components.com
MIT License
7.65k stars 380 forks source link

Not getting chunks SSR #193

Closed yyynnn closed 5 years ago

yyynnn commented 5 years ago

Hi, got a problem here.

Using ssr with ts-node ts server, and not getting any loadable chunks. Getting this in extactor:

​get -> scriptTags <script>window.__LOADABLE_REQUIRED_CHUNKS__ = [];</script>
<script async data-chunk="app" src="/bundle.app.js"></script>
​get -> linkTags <link data-chunk="app" rel="preload" as="script" href="/bundle.app.js">
​get -> scriptTags <script>window.__LOADABLE_REQUIRED_CHUNKS__ = [];</script>
<script async data-chunk="app" src="/bundle.app.js"></script>
​get -> linkTags <link data-chunk="app" rel="preload" as="script" href="/bundle.app.js">

but app loads fine and loadable components load fine too.

gregberge commented 5 years ago

Hello @yyynnn are you sure to correctly render all the tree server-side before calling getScriptTags ?

yyynnn commented 5 years ago

Yeah, ditched ts-node, now resolving ts with babel-node. Chunks still not created. I narrowed it down to this code:


import React from 'react'
import loadable from '@loadable/component'

const Fallback = () => <div>loading</div>

export const PageGenerator = ({ data }) => { //data has names of the needed components from API call
  return (
    <React.Fragment>
      {data.map(({ name, id }: ComponentProps, i) => {
        const LoadableComponent = loadable(() => import(`lazies/${name}.lazy`))
        return (
          <React.Fragment key={i}>
            <LoadableComponent id={id} />
          </React.Fragment>
        )
      })}
    </React.Fragment>
  )
}

with this one i get Uncaught Error: Module build failed (from ./node_modules/babel-loader/lib/index.js): TypeError: Property params expected type of array but got null and no chunks. I think mapping is the issue here, not so static.

On the other hand with regular dynamic import import eg in router

const Home = loadable(() => import('client/features/common/pages/Home'))

this works fine on clientside with no ssr, but fails with this error on ssr (but chunks loaded successfuly) : Uncaught ReferenceError: exports is not defined

yyynnn commented 5 years ago

First issue was resolved just by setting a function with loadable import outside of map.


const getLoadableComponent = name => {
  return loadable(() => import(`lazies/${name}.lazy`))
}

export const PageGenerator = ({ data }) => {
  return (
    <React.Fragment>
      {data.map(({ name, id }: ComponentProps, i) => {
        const Component = getLoadableComponent(name)
        return (
          <React.Fragment key={i}>
            <Component id={id} />
          </React.Fragment>
        )
      })}
    </React.Fragment>
  )
}

With second issue - still no go

gregberge commented 5 years ago

Hello @yyynnn, thanks for precision, I have better comprehension of your issue.

You are creating a dynamic component inside another one, this is an anti-pattern in React, when you do that, React will remount component each time because it is a new instance. All your components must be define statically.

I see what you want to achieve and there is an easy way to do it properly with Loadable Components:

const Loadable = loadable(props => import(`lazies/${props.name}.lazy`))

export const PageGenerator = ({ data }) => {
  return (
    <React.Fragment>
      {data.map(({ name, id }: ComponentProps) => <Loadable key={id} name={name} />)}
    </React.Fragment>
  )
}

I hope it will solve your problem!

bertho-zero commented 5 years ago

I currently have the same problem, I think the Context is not the same. Each takes one of its node_modules.

bertho-zero commented 5 years ago

https://github.com/smooth-code/loadable-components/blob/0d3e7dee3045737be1709ffa300aca083aef457c/packages/server/src/ChunkExtractor.js#L274-L276

collectChunks should be able to take a context as a second parameter, which would be passed in props to ChunkExtractorManager and used in place of the Context if defined.

And @loadable/components should export its Context like react-redux.

My server and my apps are in different packages with each their node_modules.

https://github.com/reduxjs/react-redux/blob/ab7745062b2249e3914400452a9c114b2007e969/src/components/Provider.js#L64

bertho-zero commented 5 years ago

I managed to get around the problem by doing:

//app.js
import { __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED as LoadableSecret } from '@loadable/component';

const { Context: LoadableContext } = LoadableSecret;

const element = (
  <LoadableContext.Provider value={extractor}>
    {/* */}
  </LoadableContext.Provider>
);

I use the Context directly in my apps and I pass the extractor from the server.

gregberge commented 5 years ago

I understand your issue. Be careful of building your app correctly, server-side modules must be externals. I think we should document it. See https://github.com/smooth-code/loadable-components/blob/master/examples/server-side-rendering/webpack.config.babel.js#L40

I created an issue: https://github.com/smooth-code/loadable-components/issues/231

bertho-zero commented 5 years ago

I think it's enough to export the context, as react-redux does. My problem does not come from webpack but because in my monorepo I have several packages with a @loadable/components for each, and therefore a different context.

gregberge commented 5 years ago

You should try to have a single version of the package. You will likely have some bugs with other libraries.. Are you sure to not have any workaround?

bertho-zero commented 5 years ago

I have no problem with other libraries since they expose their Context. The workaround is to monitor the versions to have only one, the ideal would be to use the Context and have a stable code in all cases.

gregberge commented 5 years ago

@bertho-zero the context is exposed (https://github.com/smooth-code/loadable-components/blob/master/packages/component/src/index.js#L16), if you want to use it, you can use it, you will not be fired 😉.

gregberge commented 5 years ago

I don't want to expose it, I want to keep the library as simple as possible for users. I don't want to expose something that is not necessary and that encourage bad practices like duplicating libraries.

bertho-zero commented 5 years ago

Yes I use it like that but I do not really want to be fired. 😕

An application can be split into several parts, and if there is no lerna or yarn workspace then the problem is inevitable, it depends on the organizations.

gregberge commented 5 years ago

OK, you got a point, I understand that it could be difficult with multiple repositories. I will expose it for advanced use cases.

gregberge commented 5 years ago

Hmm I remember why I choose to do it like that. It is the method used by styled-components: https://github.com/styled-components/styled-components/blob/master/packages/styled-components/src/secretInternals.js#L6

If a library widely used like styled-components choose this strategy, I think there is probably a reason. For now I will not expose it, maybe in future. Feel free to use the "dangerous" context if you want, it will not likely change. I think you should try to migrate your base code to avoid duplicated modules. Loadable Components is not the only library that will give you issues.