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

modify: react useage in README #50

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

0x-stan
Copy link

@0x-stan 0x-stan commented Feb 19, 2024

Modify react useage code in README.

In the original example, within a React (especially Next.js) project, data-toggle-theme may not work as expected. For instance, using data-toggle-theme="dark,light" could result in the theme always switching to dark, without the ability to switch to light.

// README.md

import { useEffect } from 'react'
import { themeChange } from 'theme-change'

useEffect(() => {
  themeChange(false)
  // 👆 false parameter is required for react project
+  return () => {
+    themeChange(false)
+  }
}, [])

@saadeghi
Copy link
Owner

Has there been a change about useEffect functionality recently that it needs a return now? Or is it about Next.js?
Because it was working good in previous versions of React 🤔

@0x-stan
Copy link
Author

0x-stan commented Feb 21, 2024

I think is react. Had test with a empty react project, it appears the same problem.

I don't know the root cause yet, but it can be seen from the log that useEffect is executed twice, and the themeChange function is also bind twice by addEventListener, so when the button is clicked, the [data-theme] is actually switched to dark, and then quickly switched back to light.

npx create-react-app test_change_theme
cd test_change_theme
npm i theme-change --save
// src/app.tsx

import { useEffect } from "react";
import { themeChange } from "theme-change";

function App() {
  useEffect(() => {
   console.log("themeChange useEffect"); // this will log twice
    themeChange(false);
    // 👆 false parameter is required for react project
  }, []);

  return (
    <div className="App">
      ...
        <button data-toggle-theme="dark,light">toggle theme</button>
      ...
    </div>
  );
}
// package.json
...
"dependencies": {
    "@testing-library/jest-dom": "^5.17.0",
    "@testing-library/react": "^13.4.0",
    "@testing-library/user-event": "^13.5.0",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "react-scripts": "5.0.1",
    "theme-change": "^2.5.0",
    "web-vitals": "^2.1.4"
  },
...

@rmuratov
Copy link

rmuratov commented Apr 9, 2024

That's probably because of the React strict mode. The existing usage example does not work under the strict mode, which is perhaps not fine.

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

Successfully merging this pull request may close these issues.

3 participants