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

[v4 beta] Server rendering not working #1972

Closed
vinpac opened this issue Sep 5, 2018 · 26 comments
Closed

[v4 beta] Server rendering not working #1972

vinpac opened this issue Sep 5, 2018 · 26 comments

Comments

@vinpac
Copy link

vinpac commented Sep 5, 2018

Environment

image

Reproduction

git clone https://github.com/vinpac/test-react-custom-cms-intl/tree/styled-v4
cd test-react-custom-cms-intl
yarn
yarn dev

Steps to reproduce

  1. Open localhost:3000
  2. See console. It will give you a Prop 'className' did not match
  3. Click on 'vagas'. The search bar will not match with the first page.

Screenshots

image

image

This doesn't happen on v3. You can check it the same repository on master branch.

@imbhargav5
Copy link
Member

imbhargav5 commented Sep 5, 2018

Hmm, taking a look now. Actually I am unable to run it. Image from Gyazo

@vinpac
Copy link
Author

vinpac commented Sep 5, 2018

Strange, I couldn't replicate this check error. Is this the only error? If so, deleting the *.stories files is fine.

@kachkaev
Copy link
Member

kachkaev commented Sep 5, 2018

Looks like I'm experience the same issue in my Next.js + TypeScript app after upgrading to styled-components v4, but I can't reproduce it in an MWE I've been trying to craft myself 🤔

@kachkaev
Copy link
Member

kachkaev commented Sep 5, 2018

I still can't reproduce the bug in an MWE, but what I see is that all cases of it appear around this kind of code:

const XXX = styled.div``;
const YYY = styled(XXX)``;

// <YYY />
index.js:2178 Warning: Prop `className` did not match. Server:
"MyFile__YYY-sc-1dfk8wt-1 jxgVKZ" Client: "MyFile__XXX-sc-1dfk8wt-0 ditbzw"

The effect this produces looks exactly like @vinpac has described.

My dependencies include:

    "next": "^7.0.0-canary.12",
    "styled-components": "^4.0.0-beta.0",

    "@zeit/next-typescript": "^1.1.1-canary.3",
    "babel-plugin-styled-components": "^1.6.2",
    "@types/styled-components": "^3.0.0",

babelrc:

{
  "presets": ["next/babel", "@zeit/next-typescript/babel"],
  "plugins": [
    [
      "babel-plugin-styled-components",
      { "ssr": true, "displayName": true, "preprocess": false }
    ]
  ],
  "env": {
    "production": {
      "plugins": [
        [
          "babel-plugin-styled-components",
          { "ssr": true, "displayName": false, "preprocess": false }
        ]
      ]
    }
  }
}

FYI: There was a duplicate styled-components package v3 in my node_modules as it was a dependency of another package I use. However, I locked to v4 via package.json / yarn and now there is only once instance of it in yarn.lock.

  // package.json
  "resolutions": {
    "styled-components": "4.0.0-beta.0"
  },

@quantizor
Copy link
Contributor

@kachkaev any chance you could post some sort of minimal reproduction repo so I can experiment?

@quantizor
Copy link
Contributor

@vinpac trying to run your project and getting this error:

TypeError: e.split is not a function
    at e.localeDataScript.e (/Users/bear/code/test-react-custom-cms-intl/src/server/intl/setup.ts:28:23)

@quantizor quantizor added the needs more info ✋ An issue that needs triaging and reproducible examples or more information to be resolved label Sep 6, 2018
@vinpac
Copy link
Author

vinpac commented Sep 6, 2018

@probablyup Just pushed a commit that will probably fix that.

Just enforcing that branch master is on styled-components v3 and the changes are on styled-v4 branch.

@quantizor
Copy link
Contributor

ah, gotcha

@quantizor
Copy link
Contributor

@vinpac hmm after looking at this, I'm not really sure why. We don't guarantee className consistency across environments unless you're using the babel plugin. Have you explored using the new babel typescript integration? https://babeljs.io/docs/en/babel-preset-typescript

@kachkaev
Copy link
Member

kachkaev commented Sep 6, 2018

@probablyup I'm using babel-preset-typescript, which is provided by @zeit/next-typescript

@kachkaev
Copy link
Member

kachkaev commented Sep 6, 2018

I was able to produce an MWE with no TypeScript involved:
https://github.com/kachkaev/styled-components-v4-ssr-mwe

What's happening is reeeeally strange though. If you yarn && yarn next on master, you'll get:

Warning: Prop `className` did not match. Server: "pages__StyledDiv2-sc-1cvwks4-1 LdUbX"
Client: "pages__StyledDiv1-sc-1cvwks4-0 ddDAFj"

You can go to another page and back and see the styles partially lost in the second div. That's the essence of the bug @vinpac has reported.

However if you remove react-apollo, react-i18next or both from packge.json and then rm -rf node_modules yarn.lock && yarn && yarn next, the bug won't reproduce any more! I have no idea how these two libraries can affect anything given that I'm not using them in the code at all 😅

I got to the MWE by deleting parts of my real app and here is the reason why those two unrelated dependencies are in package.json. Presence of both backages is a deal breaker for some reason.

This problem did not exist in my real app when I was using styled-components v3. The same here: you can downgrade it to v3 in my MWE while keeping all original dependencies and the class mismatch warning will be gone 😱

@kachkaev
Copy link
Member

kachkaev commented Sep 6, 2018

FYI: The bug is still reproducible on ^4.0.0-beta.0-2

@kachkaev
Copy link
Member

kachkaev commented Sep 6, 2018

Upgrading to next@7.0.0-canary.13 does not help either. MWE updated.

@quantizor
Copy link
Contributor

If you see that -0 and -1 on the className mismatch, I think that’s because the ordering of the components in that file is changing between server and client. Usually an issue of differing configs during transpilation

@quantizor
Copy link
Contributor

Or non-deterministic transpilation which is arguably worse 😐

@kachkaev
Copy link
Member

kachkaev commented Sep 6, 2018

Where do you think the error can be coming from? What is different in v4 compared to v3 that could make the bug possible?

Interestingly, the wrong class name is on the client, not on the server.

@quantizor
Copy link
Contributor

quantizor commented Sep 6, 2018 via email

@quantizor
Copy link
Contributor

I have absolutely no idea what's going on here. If I remove a dependency like react-apollo, it goes away. If I add it back, it comes back. So it's deterministically interdeterministic. This is really f*ing bizarre.

@quantizor
Copy link
Contributor

quantizor commented Sep 6, 2018

Okay this has been a fun debugging experience... multiple versions of hoist-non-react-statics were being used and they conflict with each other.

@quantizor
Copy link
Contributor

Adding this made it go away:

"resolutions": {
    "hoist-non-react-statics": "3.0.0"
}

quantizor added a commit to quantizor/next.js that referenced this issue Sep 6, 2018
you folks should really use semver ranges, but since greenkeeper
is no longer running for this branch things have fallen behind

in particular, there seems to be an incompatibility with multiple
v2.x versions of this package in the same bundle as displayed here:

styled-components/styled-components#1972

When I force resolution to a particular h-n-r-s version, the issue
goes away. This is true for both next 6 and next 7 canary.

But really you folks should use semver carets instead of pinning.
quantizor added a commit to quantizor/next.js that referenced this issue Sep 6, 2018
you folks should really use semver ranges, but since greenkeeper
is no longer running for this branch things have fallen behind

in particular, there seems to be an incompatibility with multiple
v2.x versions of this package in the same bundle as displayed here:

styled-components/styled-components#1972

When I force resolution to a particular h-n-r-s version, the issue
goes away. This is true for both next 6 and next 7 canary.
@kachkaev
Copy link
Member

kachkaev commented Sep 6, 2018

@probablyup OMG you are a star!!! 🌟 🎉 I'll check this tomorrow on on the weekend – had to revert upgrading to v4 for now.

quantizor added a commit to quantizor/next.js that referenced this issue Sep 6, 2018
you folks should really use semver ranges, but since greenkeeper
is no longer running for this branch things have fallen behind

in particular, there seems to be an incompatibility with multiple
v2.x versions of this package in the same bundle as displayed here:

styled-components/styled-components#1972

When I force resolution to a particular h-n-r-s version, the issue
goes away. This is true for both next 6 and next 7 canary.
quantizor added a commit to quantizor/next.js that referenced this issue Sep 6, 2018
you folks should really use semver ranges, but since greenkeeper
is no longer running for this branch things have fallen behind

in particular, there seems to be an incompatibility with multiple
v2.x versions of this package in the same bundle as displayed here:

styled-components/styled-components#1972

When I force resolution to a particular h-n-r-s version, the issue
goes away. This is true for both next 6 and next 7 canary.

But really you folks should use semver carets instead of pinning.
@vinpac
Copy link
Author

vinpac commented Sep 6, 2018

@probablyup Indeed, putting "hoist-non-react-statics": "3.0.0" on resolutions solves it.

@quantizor quantizor removed the needs more info ✋ An issue that needs triaging and reproducible examples or more information to be resolved label Sep 6, 2018
@quantizor
Copy link
Contributor

Going to leave this open until the PRs against next are merged

@vinpac
Copy link
Author

vinpac commented Sep 6, 2018

Thank you for your help!

timneutkens pushed a commit to vercel/next.js that referenced this issue Sep 7, 2018
you folks should really use semver ranges, but since greenkeeper
is no longer running for this branch things have fallen behind

in particular, there seems to be an incompatibility with multiple
v2.x versions of this package in the same bundle as displayed here:

styled-components/styled-components#1972

When I force resolution to a particular h-n-r-s version, the issue
goes away. This is true for both next 6 and next 7 canary.
timneutkens pushed a commit to vercel/next.js that referenced this issue Sep 7, 2018
you folks should really use semver ranges, but since greenkeeper
is no longer running for this branch things have fallen behind

in particular, there seems to be an incompatibility with multiple
v2.x versions of this package in the same bundle as displayed here:

styled-components/styled-components#1972

When I force resolution to a particular h-n-r-s version, the issue
goes away. This is true for both next 6 and next 7 canary.

But really you folks should use semver carets instead of pinning.
@quantizor
Copy link
Contributor

quantizor commented Sep 7, 2018

Ok looks like updates are out for both versions of next, gonna close this. Thanks @timneutkens!

@kachkaev
Copy link
Member

kachkaev commented Sep 7, 2018

Thank you for investigating this @probablyup! 🚀 I'm so keen to try styled-components v4 with Next 7.0.0-canary.14, just need to get back home for that 😅

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

No branches or pull requests

4 participants