Can't make it work with React 18

garronej commented 2 years ago

Hi @xeoneux,

Thank you for this very nice module.
I have it setup in the demo repo of tss-react with Next.js but since upgrading to React 18 I can't make it work anymore.

Observed behavior:

When reloading the page, the mode automatically switches back to OS default.

Expected behavior:

The mode to be persisted across reload.

Steps to reproduce:

git clone https://github.com/garronej/tss-react
cd tss-react
yarn
yarn build
yarn start_ssr

Setup highlights:

Video demo

https://user-images.githubusercontent.com/6702424/169627475-f3967569-4ef3-4708-aaa7-cc893b1e5376.mov

@Deckstar: Hello, would you be kind enough to confirm or infirm that next-dark-mode no longer work with React 18 + Next.js 12.1.7-canary.4 (or newer)?

Deckstar commented 2 years ago

Hey @garronej , thanks for reaching out!

I can confirm that it doesn't work for me. After refreshing the page, the mode resets.

I created a reproduction repo project where the issue can be reproduced (note that it uses yarn 3).

Edit: I didn't try with Next canary, but 12.1.6 and React 18 don't seem to work.

GIF: ezgif-4-fa62c4123e

package.json file:

{
  "name": "tss-next-dark-mode",
  "version": "0.1.0",
  "private": true,
  "scripts": {
    "dev": "next dev",
    "build": "next build",
    "start": "next start",
    "lint": "next lint"
  },
  "dependencies": {
    "@emotion/cache": "^11.7.1",
    "@emotion/react": "^11.9.0",
    "@emotion/server": "^11.4.0",
    "@emotion/styled": "^11.8.1",
    "@mui/icons-material": "^5.8.0",
    "@mui/material": "^5.8.0",
    "clsx": "^1.1.1",
    "next": "12.1.6",
    "next-dark-mode": "^3.0.0",
    "react": "18.1.0",
    "react-dom": "18.1.0",
    "tss-react": "^3.6.2"
  },
  "devDependencies": {
    "@types/node": "17.0.35",
    "@types/react": "18.0.9",
    "@types/react-dom": "18.0.4",
    "@typescript-eslint/eslint-plugin": "^5.25.0",
    "@typescript-eslint/parser": "^5.25.0",
    "eslint": "8.16.0",
    "eslint-config-next": "12.1.6",
    "eslint-import-resolver-typescript": "^2.7.1",
    "eslint-plugin-simple-import-sort": "^7.0.0",
    "typescript": "4.6.4"
  }
}

Note that I've actually moved away from using this library, so I can't comment much more.

It's an awesome library, it's just that I wanted to have static rendering on some of my Next JS pages. This isn't possible with the current cookies solution (at least not in any obvious way), so I just made the whole site use permanent dark mode 😄

garronej commented 2 years ago

@Deckstar sorry for the late reply.
Thank you so very much for taking the time to test and even create a reproduction repo, that is so kind of you!

xeoneux commented 2 years ago

Hi @garronej & @Deckstar, I have updated the example to use React 18 and that's working fine: https://next-dark-mode.vercel.app I'll still take a look at your demo repo to see what could be the issue.

garronej commented 2 years ago

Hi @xeoneux,
Thank you for having a look on a sunday night!
The test repos we provided Deckstar and I may make it look like it's problem related with @emotion, tss-react or mui.
I prepared a vanilla repo whit the latest version of React, Next and next-dark-mode where the problem can be reproduced.

git clone https://github.com/garronej/powerhooks
cd powerhooks
git checkout 88aff53703699e3c183bb1a1f63b02130ec49aa2
yarn
yarn buil
yarn start_ssr

Then it's on http://localhost:3001/
The code of the app is in src/test/ssr.

xeoneux commented 2 years ago

Hi @garronej, I tried your repo and came to this conclusion: If you build the app, then the behaviour is normal but if you run the app using yarn dev (next dev) then yes, behaviour is inconsistent. This is really weird and that's when I stumbled upon this: https://reactjs.org/blog/2022/03/08/react-18-upgrade-guide.html#updates-to-strict-mode

So the thing is, React 18 introduces a new development-only check to Strict Mode. This new check will automatically unmount and remount every component, whenever a component mounts for the first time, restoring the previous state on the second mount.

This causes inconsistent behaviour as Next.js is using strict mode by default. However, everything is fine in production mode.

garronej commented 2 years ago

I see, thank you for the explanation!

theskillwithin commented 2 years ago

Its not really useful to work with if it only works in production mode tho

garronej commented 2 years ago

Hi @theskillwithin,
If you want I have an implementation based on powerhooks that is fully compatible with React18 and works like this module.

It even does a bit more by switching the root color scheme which make the scroll bars the correct color and you can select the theme by adding ?theme=light, ?theme=dark in the URL.

@xeoneux I don't mean to disrespect your work by suggesting my own implementation, feel free to delete my message. I allowed myself to suggest my alternative because I assumed that you had no real intention of addressing this point.

xeoneux / next-dark-mode