Pass an HOC to renderPage() #2010

jcheroske · 2017-05-18T19:39:17Z

There has been some discussion at #1751 and #1776 about changing the renderPage() signature to better support SSR and allow wrapping of all pages in the application. My suggestion is to simply allow an HOC to be passed into renderPage().

mxstbr · 2017-05-27T07:50:31Z

This would be amazing to merge as it'd make SSR with styled-components and Next.js much nicer!

timneutkens

I don't see any harm in adding this 👌

jcheroske · 2017-05-27T14:07:29Z

Awesome! I'll work up a JSS example.

…

On Sat, May 27, 2017 at 4:22 AM Tim Neutkens ***@***.***> wrote: ***@***.**** approved this pull request. I don't see any harm in adding this 👌 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2010 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADgUyq5FMX5XXBTvlubKB78wOYAO6uP_ks5r-AeTgaJpZM4Nfs5p> .

mxstbr · 2017-06-13T18:38:40Z

Can we land this maybe? 😊 @arunoda @rauchg

Folks have been asking us about the best way to do Next + styled-components, this would make it so much better!

jcheroske · 2017-06-13T18:40:30Z

Is this waiting on me at all? I'm swamped and can't do the docs right now.

…

On Tue, Jun 13, 2017 at 11:38 AM, Max Stoiber ***@***.***> wrote: Can we land this maybe? 😊 @arunoda <https://github.com/arunoda> @rauchg <https://github.com/rauchg> Folks have been asking us about the best way to do Next + styled-components, this would make it so much better! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2010 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADgUyiqenuQqELZdrEKRoy8cw-dfRYceks5sDtc-gaJpZM4Nfs5p> .

timneutkens · 2017-06-13T19:07:42Z

@jcheroske no it's us 😉 You did a great job.

jcheroske · 2017-06-13T19:26:21Z

Whew! For once, it's not my fault.

Oh, and sorry about the .gitignore change. I realized too late that is bad form.

timneutkens · 2017-06-28T18:02:54Z

@arunoda lets merge this if you agree 👌

iamstarkov

very nice to have feature

iamstarkov · 2017-07-12T22:20:09Z

@arunoda @timneutkens can we make it to be merged and released? it makes cssinjs's ssr so much easier

iamstarkov · 2017-07-17T08:34:28Z

@jcheroske @mxstbr @kof @timneutkens just fyi https://twitter.com/arunoda/status/885645093436141568

On it. After 3.0
— Arunoda Susiripala (@arunoda) July 13, 2017

timneutkens · 2017-07-17T09:50:32Z

👌 thanks @iamstarkov I saw the tweet 😄

kof · 2017-07-23T11:45:59Z

.gitignore

@@ -13,3 +13,4 @@ npm-debug.log
 coverage

 .DS_Store
+.idea


@jcheroske this is unrelated, sometimes PRs are not merged because of things like this.

I've removed it ❤️

kof · 2017-07-23T11:47:39Z

@jcheroske can you please remove .gitgnore change and add a test? Otherwise it seems to be fine.

jcheroske · 2017-07-23T13:15:31Z

No problem.

…

On Sun, Jul 23, 2017 at 4:47 AM Oleg Slobodskoi ***@***.***> wrote: @jcheroske <https://github.com/jcheroske> can you please remove .gitgnore change and add a test? Otherwise it seems to be fine. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2010 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADgUyilxxgc2vCPjrou58hsR3OD6rfRoks5sQzLigaJpZM4Nfs5p> .

iamstarkov · 2017-08-08T18:10:44Z

next 3 is released, so i hope we can expect this to be merged soon =)

jcheroske · 2017-08-09T15:44:48Z

Totally. My git skills are not very strong. I was hesitant to make that change. Thanks for taking care of it.

…

On Wed, Aug 9, 2017 at 8:43 AM Tim Neutkens ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In .gitignore <#2010 (comment)>: > @@ -13,3 +13,4 @@ npm-debug.log coverage .DS_Store +.idea I've removed it ❤️ — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2010 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADgUysDPF5TNSfMOS-SCv8byfXyQbhfZks5sWdOSgaJpZM4Nfs5p> .

timneutkens · 2017-08-13T13:43:37Z

@arunoda this is ready to be merged, I guess tests should be added for it though 🤔

mxstbr · 2017-08-21T19:26:28Z

Can we merge this? 😊 It's been a while and it would make styled-components + Next soooo much better!

timneutkens

+1 👌

timneutkens · 2017-08-21T21:15:25Z

@mxstbr done 👍, will be included in the Next release.

mxstbr · 2017-08-22T06:28:15Z

AWESOME thank you! I'll send a PR to update the official example and we'll get our docs in sync.

mxstbr · 2017-08-22T07:12:01Z

Hmm, I'm trying to update the example locally but I'm getting "The default export is not a React Component in page: "/""

This is the _document.js:

export default class MyDocument extends Document {
  static getInitialProps ({ renderPage, req }) {
    const isServer = !!req;
    if (isServer) {
      const sheet = new ServerStyleSheet()
      // I don't know why I have to .bind here, but otherwise this is undefined in sheet.collectStyles
      const page = renderPage(sheet.collectStyles.bind(sheet))
      const styleTags = sheet.getStyleTags()
      return { ...page, styleTags }
    }
  }

  render () {
    return (
      <html>
       <Head>
         <title>Next with styled-components</title>
         {this.props.styleTags}
       </Head>
       <body>
         <Main />
         <NextScript />
       </body>
     </html>
    )
  }
}

Any ideas why the component could not be a function?

mxstbr · 2017-08-22T07:16:09Z

I tried only importing the ServerStyleSheet on the server but that didn't work either:

  static getInitialProps ({ renderPage, req }) {
    const isServer = !!req;
    if (isServer) {
      const ServerStyleSheet = eval("require('styled-components').ServerStyleSheet")
      const sheet = new ServerStyleSheet()
      const page = renderPage(sheet.collectStyles.bind(sheet))
      const styleTags = sheet.getStyleTags()
      return { ...page, styleTags }
    }
    return {};
  }

Based on vercel#2010 this updates the styled-components to use the new renderPage() signature. Note that this doesn't work yet, it throws an error.

mxstbr · 2017-08-22T07:23:11Z

Moving discussion to: #2831

iamstarkov · 2017-08-22T15:47:39Z

yay 🎉

fresh5447 · 2017-08-22T16:49:23Z

Awesome!

arunoda · 2017-08-30T10:29:58Z

This is awesome.

@jcheroske I'm gonna do a release now.
Could you add a section about this on the README? I think it'll be useful.

arunoda · 2017-08-30T10:35:12Z

Check next@3.1.0

jcheroske · 2017-08-30T12:07:30Z

I would be happy to do that.

…

On Wed, Aug 30, 2017 at 3:35 AM Arunoda Susiripala ***@***.***> wrote: Check ***@***.*** — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2010 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADgUyqRgPn_mRH0qI1TllfMKjbeBcZOUks5sdTrogaJpZM4Nfs5p> .

NikitaVlaznev · 2018-06-27T02:05:24Z

As i can see, renderPage enhancer runs only on the server side, so if it will wrap the page with any tag then there will be a react tree mismatch after browser rendering with unpredictable behavior and some warning from react like "expected server HTML to contain a matching ...", isn't it?

timneutkens · 2018-06-27T08:36:56Z

This feature (wrapping the App) was introduced to support css-in-js libraries like styled-components. You shouldn't use it to wrap extra layout, that's what _app.js is for.

NikitaVlaznev · 2018-06-27T11:17:09Z

Tricky code. It is worth to mention that actually the App is wrapping this enhancer, and the enhancer is wrapping the Page, but only during server side rendering.

JSS example for material-ui still offers to wrap every page manually to handle the styles.
Here is a PR of how it can be done using the App: mui/material-ui#11989.

It is based on passing pageContext from the App down to the enhancer and then to the Document.
Still looks tricky but should work.

jcheroske · 2018-07-05T20:44:50Z

Sorry, I never did get around to writing docs for this. I'm no longer using next.js, or doing client-side work atm. I hope the project is still healthy.

kof · 2018-07-05T21:03:34Z

I don't use next.js yet either, thats the problem, waiting for someone who uses both.

Added passing an HOC to renderPage()

8945a36

This was referenced May 18, 2017

Wrapping App and injecting styles into the head in _document #1751

Closed

Added transformPage function to renderPage to allow custom wrapper on… #1776

Closed

timneutkens requested review from rauchg and arunoda May 27, 2017 11:20

timneutkens approved these changes May 27, 2017

View reviewed changes

sedubois mentioned this pull request May 28, 2017

Prevent remounting of layout component on every page? #2091

Closed

Merge branch 'master' into master

6412e7d

timneutkens added Status: Ready and removed Status: Ready labels Jul 1, 2017

jcheroske mentioned this pull request Jul 12, 2017

Add an example to next.js cssinjs/jss#457

Closed

kof approved these changes Jul 12, 2017

View reviewed changes

iamstarkov approved these changes Jul 12, 2017

View reviewed changes

kof reviewed Jul 23, 2017

View reviewed changes

Remove gitignore

b2ff74e

timneutkens assigned arunoda Aug 13, 2017

timneutkens approved these changes Aug 21, 2017

View reviewed changes

timneutkens merged commit 9c18c54 into vercel:master Aug 21, 2017

mxstbr added a commit to mxstbr/next.js that referenced this pull request Aug 22, 2017

WIP: Update styled-components example

74ac63a

Based on vercel#2010 this updates the styled-components to use the new renderPage() signature. Note that this doesn't work yet, it throws an error.

mxstbr mentioned this pull request Aug 22, 2017

WIP: Update styled components example #2831

Closed

kitten mentioned this pull request Aug 29, 2017

Upgrade next to version 3 as soon as 3.0.7 is released styled-components/styled-components-website#94

Closed

oliviertassinari mentioned this pull request Aug 30, 2017

[SSR] Remove the singleton hack ✨ mui/material-ui#7965

Merged

3 tasks

timneutkens mentioned this pull request Nov 12, 2017

Combining _document with redux connect or apollo #1970

Closed

lock bot locked as resolved and limited conversation to collaborators Jul 7, 2019

Pass an HOC to renderPage() #2010

Pass an HOC to renderPage() #2010

Conversation

jcheroske commented May 18, 2017

mxstbr commented May 27, 2017

timneutkens left a comment

Choose a reason for hiding this comment

jcheroske commented May 27, 2017 via email

mxstbr commented Jun 13, 2017

jcheroske commented Jun 13, 2017 via email

timneutkens commented Jun 13, 2017

jcheroske commented Jun 13, 2017 • edited

timneutkens commented Jun 28, 2017

iamstarkov left a comment

Choose a reason for hiding this comment

iamstarkov commented Jul 12, 2017

iamstarkov commented Jul 17, 2017 • edited

timneutkens commented Jul 17, 2017

kof Jul 23, 2017

Choose a reason for hiding this comment

timneutkens Aug 9, 2017

Choose a reason for hiding this comment

kof commented Jul 23, 2017

jcheroske commented Jul 23, 2017 via email

iamstarkov commented Aug 8, 2017

jcheroske commented Aug 9, 2017 via email

timneutkens commented Aug 13, 2017

mxstbr commented Aug 21, 2017

timneutkens left a comment

Choose a reason for hiding this comment

timneutkens commented Aug 21, 2017 • edited

mxstbr commented Aug 22, 2017

mxstbr commented Aug 22, 2017 • edited

mxstbr commented Aug 22, 2017

mxstbr commented Aug 22, 2017

iamstarkov commented Aug 22, 2017

fresh5447 commented Aug 22, 2017

arunoda commented Aug 30, 2017

arunoda commented Aug 30, 2017

jcheroske commented Aug 30, 2017 via email

NikitaVlaznev commented Jun 27, 2018 • edited

timneutkens commented Jun 27, 2018

NikitaVlaznev commented Jun 27, 2018 • edited

jcheroske commented Jul 5, 2018

kof commented Jul 5, 2018

jcheroske commented Jun 13, 2017 •

edited

iamstarkov commented Jul 17, 2017 •

edited

timneutkens commented Aug 21, 2017 •

edited

mxstbr commented Aug 22, 2017 •

edited

NikitaVlaznev commented Jun 27, 2018 •

edited

NikitaVlaznev commented Jun 27, 2018 •

edited