preactjs / preact-compat

ATTENTION: The React compatibility layer for Preact has moved to the main preact repo.
http://npm.im/preact-compat
MIT License
949 stars 148 forks source link

Server side rendering issues [aliases not working] #299

Open hanford opened 7 years ago

hanford commented 7 years ago

Hello! Thanks again for making such an amazing little framework. preact-compat has been a dream to work with, and I'm excited to integrate. I've just run into one little issue, that I hope isn't a blocker, because the performance gains are tremendous! 🔥

I'll add as much code as possible, if it's relevant, note that our application runs fine in the browser, where this code isn't executed.

So we server side render everything with React and things work fine. The code looks like this:

function renderReact (path, preloadedProps, callback) {
  return match({ routes: routes(noop, noop), location: path }, (err, redirect, props) => {
    if (err) return callback(err)

    const html = ReactDOMServer.renderToString(
      <EazeProvider path={path} renderOnServer={serverRenderedRoutes.some((route) => matchesLocation(route, path))}>
        <RouterContext
          {...props}
          createElement={(Component, props) => <Component {...props} preloadedProps={preloadedProps} />}
        />
      </EazeProvider>
    )

    let head = Helmet.rewind()

    const doc = base
      .replace(/<div id='{0,1}app'{0,1}><\/div>/, `<div id='app'>${html}</div>`)
      .replace(/<head>(.+)<\/head>/, (_, contents) => {
        return `
          <head ${head.htmlAttributes.toString()}>
            ${head.title.toString()}
            ${head.meta.toString()}
            ${head.link.toString()}
            ${contents}
          </head>
        `
      })

    return callback(null, doc)
  })
}

So we're using react-helmet, for the tags, however I don't think that's the issue. This runs fine in a node environment, and returns HTML when the function is invoked in a purely react environment (without the preact aliasing). However, when I introduced the preact & webpack aliasing, I started seeing this error:

Dynamic page loading failed!  { Invariant Violation: React.Children.only expected to receive a single React element child.
    at invariant (/Users/hanford/code/maple/render/node_modules/fbjs/lib/invariant.js:44:15)
    at Object.onlyChild [as only] (/Users/hanford/code/maple/render/node_modules/react/lib/onlyChild.js:33:84)
    at Provider.render (/Users/hanford/code/maple/render/node_modules/react-redux/lib/components/Provider.js:55:28)
    at renderToString (/Users/hanford/code/maple/render/node_modules/preact-render-to-string/dist/index.js:151:18)
    at renderToString (/Users/hanford/code/maple/render/node_modules/preact-render-to-string/dist/index.js:229:15)
    at Object.renderToString (/Users/hanford/code/maple/render/node_modules/preact-render-to-string/dist/index.js:158:11)
    at /Users/hanford/code/maple/render/index.js:9167:33
    at /Users/hanford/code/maple/render/node_modules/react-router/lib/match.js:65:5
    at /Users/hanford/code/maple/render/node_modules/react-router/lib/createTransitionManager.js:118:11
    at done (/Users/hanford/code/maple/render/node_modules/react-router/lib/AsyncUtils.js:79:19)
    at /Users/hanford/code/maple/render/node_modules/react-router/lib/AsyncUtils.js:85:7
    at /Users/hanford/code/maple/render/index.js:3872:12
    at process._tickCallback (internal/process/next_tick.js:103:7) name: 'Invariant Violation', framesToPop: 1 }

So, investigating this stack trace, the first error points to react/lib/onlyChild which is this function:

function onlyChild(children) {
  !ReactElement.isValidElement(children) ? process.env.NODE_ENV !== 'production' ? invariant(false, 'React.Children.only expected to receive a single React element child.') : _prodInvariant('143') : void 0;
  return children;
}

weirdly, the children in this argument are we're failing the ternary ReactElement.isValidElement, here is what the children arg is:

[ VNode {
    '0': [Circular],
    nodeName:
     { [Function]
       displayName: 'RouterContext',
       propTypes: [Object],
       getDefaultProps: [Object],
       childContextTypes: [Object],
       defaultProps: [Object] },
    attributes:
     { createElement: [Function: createElement],
       routes: [Object],
       params: {},
       location: [Object],
       components: [Object],
       history: [Object],
       router: [Object],
       matchContext: [Object] },
    children: undefined,
    key: undefined,
    preactCompatUpgraded: true,
    preactCompatNormalized: true,
    length: 1 } ]

After doing some investigation, commenting out the onlyChild ternary fixes server side rendering, however I don't think forking react-redux and react is the best solution here.

the ReactElement call appears to be checking for $$typeof: Symbol(react.element)

Annnnd the whole reason any of this is called is because we're rendering <Provider /> from react-redux

hanford commented 7 years ago

I was able to fix this issue by forking react-redux and modifying it to this: https://github.com/hanford/react-redux/commit/7d6a542e7b77a72949fdc78833bd32a7b9088609

developit commented 7 years ago

Hi there! It looks like your aliasing on the server is actually not taking effect - you can tell because there are React internals included in the stack trace you posted. It's likely you're using Preact + Preact-compat to render your app, but the libraries you're pulling in are actually using React.

Can you give any details about how you're doing the aliasing on the server?

For what it's worth, the best (maybe only) way I'm aware of to properly alias preact-compat under node (without webpack) is module-alias:

require('module-alias').addAliases({
  'react'  : 'preact-compat',
  'react-dom': 'preact-compat'
});

or, specify this in your package.json:

"_moduleAliases": {
  "react": "preact-compat",
  "react-dom": "preact-compat"
}

... and install the hook:

require('module-alias/register');
mikestead commented 7 years ago

Not sure if this will help you but...

We have this compiling on the server with webpack and had to make sure to exclude the correct modules from externals e.g.

// webpack.server.js
const config = {
    ...
    target: 'node',
    node: {
        __dirname: false,
        __filename: false
    },
   externals: getExternals()
   ...
}

function getExternals() {
    const excludes = [ 'react-router', 'react-redux', 'react' ];
    return fs.readdirSync('node_modules')
        .filter((x) => !~excludes.indexOf(x))
        .reduce((nodeModules, mod) => {
             nodeModules[mod] = 'commonjs ' + mod;
             return nodeModules;
         }, {});
}

Without the excludes list, webpack won't process those libs, so they don't get aliased by preact. react is obviously the key one here, add others as needed.

developit commented 7 years ago

@mikestead are you aliasing outside webpack to handle that then? interested to know how that works.

mikestead commented 7 years ago

Nope just aliasing in the webpack config as usual.

config.resolve.alias = {
    react: 'preact-compat',
    'react-dom': 'preact-compat'
}

Because webpack picks up the alias and is processing react imports it seems to swap out react for preact compat, just like for the client, and all seems to work fine.

developit commented 7 years ago

nice!

developit commented 7 years ago

@hanford where did we end up with this? Definitely looked like an aliasing issue to me (the fact that react/* was being included at all is basically a guarantee something is amiss there).

Alternatively - you might actually just be able to alias react-redux to preact-redux - the latter is pre-aliased with a few things stripped out, it'll both reduce your bundle size and fix the children.only issue.

bradennapier commented 7 years ago

Yeah, cant make this work - every single module says it can not find react. That function gives webpack 2 a bunch of errors as well

I have added to package.json, added to aliasing for webpack, added everything. react-helmet wasnt working so i stopped using it then I started getting react-router errors which i obviously am not about to remove and re-do the entire app for :(

developit commented 7 years ago

@bradennapier If anything is saying it can't find React, that means your aliases are not being used. Can you elaborate on how you did the aliasing? One common mistake I see is that people alias via Webpack, but then run their code without bundling directly in Node (which doesn't know anything about Webpack aliases).

bradennapier commented 7 years ago

Yeah, my problem was that that was where I was looking but I had that part correct. Just figured it out. I had a few modules being built into a vendor dll so when we moved into the client they were not aliased.

developit commented 7 years ago

ahhh yeah, that sounds about right. if there was something funky required to alias that DLL by all means post it here. I think we need to add some details to the aliasing section of the preact website to cover these cases.

bradennapier commented 7 years ago

I just removed them all together, it was only for development anyway so I am using the dll for lodash and friends only.

developit commented 7 years ago

Ahh, ok. I think we can probably safely say webpack DLL's are not aliasable, that's at least good to know.

thomas-p-wilson commented 7 years ago

Am having the same issue here, and I referred to #242 in which I found a quick solution to get requires working correctly. As evidenced in the following SSR stacktrace, you can see that preact is being substituted for react as expected, and yet I am encountering the same issue.

Invariant Violation: React.Children.only expected to receive a single React element child.
    at invariant (/usr/src/app/node_modules/fbjs/lib/invariant.js:44:15)
    at Object.onlyChild [as only] (/usr/src/app/node_modules/react/lib/onlyChild.js:33:84)
    at Provider.render (/usr/src/app/node_modules/react-redux/lib/components/Provider.js:51:28)
    at renderToString (/usr/src/app/node_modules/preact-render-to-string/src/index.js:90:18)
    at renderToString (/usr/src/app/node_modules/preact-render-to-string/src/index.js:97:11)
    at renderToString (/usr/src/app/node_modules/preact-render-to-string/src/index.js:97:11)
    at /usr/src/app/src/js/server/core/middleware/post/react-server.js:46:30
    at run (/usr/src/app/node_modules/babel-polyfill/node_modules/core-js/modules/es6.promise.js:87:22)
    at /usr/src/app/node_modules/babel-polyfill/node_modules/core-js/modules/es6.promise.js:100:28
    at flush (/usr/src/app/node_modules/babel-polyfill/node_modules/core-js/modules/_microtask.js:18:9)
    at _combinedTickCallback (internal/process/next_tick.js:73:7)
    at process._tickDomainCallback (internal/process/next_tick.js:128:9)

Am about to try with preact-redux as suggested above. Will advise on my progress.

    "preact": "7.2.x",
    "preact-compat": "3.14.x",
    "react-redux": "4.4.x",
    "react-router": "2.x.x",
    "redux": "3.3.x"
thomas-p-wilson commented 7 years ago

Problem solved with preact-redux!

mgutz commented 7 years ago

Still seeing this issue. The easiest workaround is to symlink preact-compact to react and react-dom in node_modules.

sedlukha commented 4 years ago
"_moduleAliases": {
  "react": "node_modules/preact/compat/dist/compat.js",
  "react-dom": "node_modules/preact/compat/dist/compat.js"
}

solved my issue on server side (preact 10.4.0)

there was a problem with using react-redux on server

developit commented 4 years ago

Yes, for Node/SSR module-alias is a good solution with the configuration @sedlukha posted.

rodynenko commented 4 years ago

@sedlukha Can you help how to patch preact-cli@3.0.0-rc.10 with module-alias? where is the place for require('module-alias/register')? Thanks

sedlukha commented 4 years ago

@rodynenko I don't use preact-cli. You need to add require('module-alias/register') at the very main file of server side of your app, before any code.

In my case I use razzle for ssr. So my target file is server.js.(preact example)

marvinhagemeister commented 4 years ago

@rodynenko This shouldn't be needed for preact-cli it has the alias properties already set up in webpack.

rodynenko commented 4 years ago

@sedlukha @marvinhagemeister Thanks. It seems to be en error in my code.

hanford commented 4 years ago

Is this resolved? No longer relevant to me / happy to close if it's resolved