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

Bug: componentWillUnmount is called twice #24280

Closed
mgomulak opened this issue Apr 5, 2022 · 12 comments
Closed

Bug: componentWillUnmount is called twice #24280

mgomulak opened this issue Apr 5, 2022 · 12 comments
Labels
React 18 Bug reports, questions, and general feedback about React 18 Type: Bug

Comments

@mgomulak
Copy link

mgomulak commented Apr 5, 2022

React version: 18.0.0

Steps To Reproduce

componentWillUnmount is called twice upon toggling the rendered component. Even when StrictMode is disabled

Link to code example: https://codesandbox.io/s/componentwillunmount-called-twice-hrpzy5?file=/src/App.js

The current behavior

After upgrading to react 18 we've seen some different behavior in a conditionally rendered, lazy class component.

In the provided code example the class component is rendered first. After the first toggle, the class component's componentWillUnmount is called twice.

Subsequent toggle calls correctly lead to a single componentWillUnmount invocation.

This does only seem to affect the class component when its rendered first. If the condition is changed to initially show the other function component the class component unmounts just fine

The expected behavior

The class component's componentWillUnmount is only called once

@mgomulak mgomulak added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Apr 5, 2022
@gaearon
Copy link
Collaborator

gaearon commented Apr 5, 2022

Woah.

@gaearon gaearon added Type: Bug React 18 Bug reports, questions, and general feedback about React 18 and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Apr 5, 2022
@Kramb
Copy link

Kramb commented Apr 6, 2022

I believe that this is a similar problem that I am having with the useEffect hook.
I am providing a sample on StackBlitz to illustrate my problem. I apologize if this is not the appropriate place to put this and I will open a new Issue if it is necessary.

useEffect Multiple Calls

@gaearon
Copy link
Collaborator

gaearon commented Apr 6, 2022

@Kramb, no, the duplicate matching useEffect setup+cleanup calls are expected in 18 when Strict Mode is on.
See https://reactjs.org/blog/2022/03/08/react-18-upgrade-guide.html#updates-to-strict-mode

This issue is about an unrelated bug where componentWillUnmount does not "match" componentDidMount when used inside of <Suspense>.

@Kramb
Copy link

Kramb commented Apr 6, 2022

@gaearon Okay, I see now. I appreciate the clarification. I've been running in circles with this one and I think I jumped at the first thing that resembled my issue.

@gaearon
Copy link
Collaborator

gaearon commented Apr 6, 2022

No problem! Did you miss it in the upgrade post, or did you update without reading in detail? We’re trying to figure out how to surface this more prominently so it would be helpful to know more about how people run into this.

@Kramb
Copy link

Kramb commented Apr 6, 2022

I did not even notice there was an update to react because I used CRA to start a new project. When I made a call to my API I noticed that it was making the call twice in the "Network" tab of the developer's console.
I hope this helps and I would be more than happy to provide you with additional information should you need it.

@gaearon
Copy link
Collaborator

gaearon commented Apr 11, 2022

The componentWillUnmount issue prompted a significant refactor and was fixed in #24308. We'll be testing this internally.

For future googlers, I want to reiterate that double-firing effects is not related to this issue. See #24280 (comment) for explanation of that behavior.

@gaearon gaearon closed this as completed Apr 11, 2022
@mgomulak
Copy link
Author

Thanks 🚀

@gaearon
Copy link
Collaborator

gaearon commented Apr 27, 2022

Thank you for reporting this. This should be fixed in 18.1.0.

@mgomulak
Copy link
Author

Yep can confirm, that after upgrading to 18.1.0 we're no longer able to reproduce the problem. Thanks again 🙏

@gunaygultekin
Copy link

gunaygultekin commented Apr 28, 2022

unfortunately, the problem exists. Here is my package.json

{
  "name": "app",
  "version": "0.1.0",
  "private": true,
  "homepage": ".",
  "dependencies": {
    "@heroicons/react": "^1.0.6",
    "@reduxjs/toolkit": "^1.8.1",
    "axios": "^0.27.2",
    "i18next": "^21.6.16",
    "moment": "^2.29.2",
    "react": "^18.1.0",
    "react-dom": "^18.1.0",
    "react-i18next": "^11.16.6",
    "react-redux": "^7.2.8",
    "react-router-dom": "^6.2.1",
    "react-scripts": "5.0.0",
    "typescript": "^4.6.3"
  },
  "scripts": {
    "start": "react-scripts start",
    "build:dev": "env-cmd -f .env.dev react-scripts build",
    "build:test": "env-cmd -f .env.test react-scripts build",
    "test": "react-scripts test",
    "test:coverage": "react-scripts test --coverage --watchAll=false",
    "test:debug": "react-scripts --inspect-brk test --runInBand --no-cache",
    "eject": "react-scripts eject"
  },
  "jest": {
    "collectCoverageFrom": [
      "src/**/*.{js,jsx,ts,tsx}",
      "!<rootDir>/node_modules/",
      "!<rootDir>/path/to/dir/",
      "!src/**/*.d.ts",
      "!<rootDir>/src/ts/**",
    ],
    "coverageThreshold": {
      "global": {
        "branches": 70,
        "functions": 70,
        "lines": 70,
        "statements": 70
      }
    },
    "coverageReporters": [
      "html"
    ]
  },
  "eslintConfig": {
    "extends": "react-app"
  },
  "browserslist": {
    "production": [
      ">0.2%",
      "not dead",
      "not op_mini all"
    ],
    "development": [
      "last 1 chrome version",
      "last 1 firefox version",
      "last 1 safari version"
    ]
  },
  "devDependencies": {
    "@testing-library/jest-dom": "^5.16.4",
    "@testing-library/react": "^13.1.1",
    "@testing-library/user-event": "^14.1.1",
    "@types/jest": "^27.4.1",
    "@types/node": "^17.0.29",
    "@types/react": "^18.0.8",
    "@types/react-dom": "^18.0.0",
    "@types/react-redux": "^7.1.23",
    "autoprefixer": "^10.4.4",
    "cssnano": "^5.1.7",
    "env-cmd": "^10.1.0",
    "eslint-plugin-react-hooks": "^4.4.0",
    "node-sass": "^7.0.1",
    "postcss": "^8.4.12",
    "postcss-import": "^14.1.0",
    "prettier": "^2.6.2",
    "tailwindcss": "^3.0.23"
  },
  "engines": {
    "npm": ">=8.1.0",
    "node": ">=14.18.1"
  }
}

@gaearon
Copy link
Collaborator

gaearon commented Apr 28, 2022

@gunaygultekin

Please read #24280 (comment). I suspect the problem you’re facing is not related to this issue which has been fixed. If you found some different problem from either of these two please file a new issue.

I will lock this issue because I expect more people to be confused in a similar way.

@facebook facebook locked as resolved and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
React 18 Bug reports, questions, and general feedback about React 18 Type: Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants