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: deprecated findDOMNode usage with React.StrictMode #20972

Closed
t18n opened this issue Apr 5, 2020 · 12 comments · Fixed by #21932
Closed

bug: deprecated findDOMNode usage with React.StrictMode #20972

t18n opened this issue Apr 5, 2020 · 12 comments · Fixed by #21932
Labels
package: react @ionic/react package

Comments

@t18n
Copy link

t18n commented Apr 5, 2020

Bug Report

Ionic version:
[x] 4.x
"@ionic/react": "^5.0.7"

Current behavior:

I am trying to install Ionic React to a fresh create-react-app but hit the error

findDOMNode is deprecated in StrictMode. findDOMNode was passed an instance of IonComponent which is inside StrictMode
quite a lot. I notice that if I create the app with ionic start, it won't notice because the <App /> component is not wrapped inside React.StrictMode. Does that mean that you guys don't support StrictMode right now, what is the plan for Ion components to work with StrictMode?

Expected behavior:

Avoid using findDOMNode to refer to an element, hence not getting this error on React Strict mode

Steps to reproduce:

  1. Create a React app with create-react-app
  2. Make sure <App /> component is wrapped inside React.StrictMode
  3. Create a component with IonReactRouter
import React, { FC } from 'react';
import { IonApp, IonRouterOutlet } from '@ionic/react';
import { IonReactRouter } from '@ionic/react-router';
import { Redirect, Route } from 'react-router-dom';
import { Home } from './pages/home';
import { About } from './pages/about';
import { Error } from './pages/error';

export const Router: FC = ({ children }) => (
	<IonApp>
		<IonReactRouter>
			<IonRouterOutlet>
				<Route path="/" exact component={Home} />
				<Route path="/about" exact component={About} />
				<Route path="/404" exact component={Error} />
				<Route path="/contact" render={() => <Redirect to="/error" />} exact={true} />
			</IonRouterOutlet>
			{children}
		</IonReactRouter>
	</IonApp>
);

or

import React from 'react';
import { IonTabBar, IonIcon, IonLabel, IonTabButton } from '@ionic/react';
import { home, callOutline, informationCircleOutline } from 'ionicons/icons';

export const Tabbar = () => (
	<IonTabBar slot="bottom">
		<IonTabButton tab="home" href="/home">
			<IonIcon icon={home} />
			<IonLabel>Home</IonLabel>
		</IonTabButton>
		<IonTabButton tab="about" href="/about">
			<IonIcon icon={informationCircleOutline} />
			<IonLabel>About</IonLabel>
		</IonTabButton>
		<IonTabButton tab="contact" href="/contact">
			<IonIcon icon={callOutline} />
			<IonLabel>Contact</IonLabel>
		</IonTabButton>
	</IonTabBar>
);
  1. Run yarn start
  2. Check browser console

image

@ionitron-bot ionitron-bot bot added the triage label Apr 5, 2020
@liamdebeasi liamdebeasi added the package: react @ionic/react package label Apr 6, 2020
@ionitron-bot ionitron-bot bot removed the triage label Apr 6, 2020
@kiily
Copy link

kiily commented Apr 10, 2020

I can reproduce this error exactly in a clean repo. I created it with CRA (which now wraps the app in React.StrictMode and this immediately fails. I followed the implementation found here: https://dev.to/ionic/adding-ionic-react-to-an-existing-react-project-4kib

I have isolated this in a template repo, https://github.com/kiily/react-ts-pwa-ionic-capacitor, you can checkout the feat-ionic-react branch run yarn install and yarn start and the warning should be visible in the DevTools console.

Additionally feel free to use this as a template to reproduce issues related to CRA apps using Ionic React and Capacitor (there were added manually instead of creating the app with the Ionic CLI).

@peterennis
Copy link
Contributor

Related: ant-design/ant-design#22493

@vimaleurakaa
Copy link

+1 Happens same thing to me

@peterpeterparker
Copy link
Contributor

The same error in material-ui: mui/material-ui#13394

@GabrielMajeri
Copy link

The problematic line is in createComponent.tsx:

const node = ReactDom.findDOMNode(this) as HTMLElement;

It uses the findDOMNode API, which is discouraged from use.

@mlynch
Copy link
Contributor

mlynch commented Sep 12, 2020

Just acknowledging we're aware of this issue. Just hit it myself adding Ionic React to an existing React app. Technically, it's a warning as functionality is not impacted, but regardless we need to fix it to avoid the future breaking change.

@peterpeterparker
Copy link
Contributor

@mlynch it's been two months since your msg and seven since the issue was opened, therefore, may I ask what's the status? is there any workaround? something blocker? any hints?

@t18n
Copy link
Author

t18n commented Nov 12, 2020

Sorry that I have missed some communications here. My workaround was to not use React in Strict mode. :(

@bard
Copy link

bard commented Nov 22, 2020

I see there's been for a couple of months a PR pending review which claims to fix the issue. Is anything blocking it? It's really easy to miss important things in the console when it's being spammed by the findDOMNode errors.

@GabrielMajeri
Copy link

The PR (#21932) has been merged 🎉

@elylucas
Copy link
Contributor

elylucas commented Nov 25, 2020

Thanks all for your patience on this one, and thanks @GabrielMajeri for the pull request! The fix will be in the next release.

@ionitron-bot
Copy link

ionitron-bot bot commented Dec 25, 2020

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Dec 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: react @ionic/react package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants