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

Reduce number of StrictMode warnings #6287

Closed
wants to merge 4 commits into from
Closed

Reduce number of StrictMode warnings #6287

wants to merge 4 commits into from

Conversation

jeffchheng
Copy link

To alleviate some of #6060.

cWRP was easy enough to change. Some instances of cWM weren’t quite as easy due to the legacy context API and having no familiarity with it.

Still, I was able to run this symlinked to my company’s app and see at least a reduction in StrictMode errors with no change in app functionality.

Also added a StrictMode test for each component, skipped if it wasn’t fixed yet.

@mjackson
Copy link
Member

Thanks for the PR, @jeffchheng :)

I did some major refactoring over the past week and eliminated all the deprecated componentWill methods in the process, so we won't need this anymore. You can see some of my commits here and here.

My apologies for not guiding you through this work so we could get this merged, but it was a little tricky due to the changes in the context API and I wanted to make sure I understood all of the implications of making this change, so I decided to do it myself. You can expect this to be published in our next beta release sometime this week. 😅

@mjackson mjackson closed this Sep 25, 2018
@jaredpalmer
Copy link
Contributor

Just a note here. This is unfortunately not enough to work within Suspense. We still need to get rid of all usage of the legacy context API for RR4 to work within <StrictMode>.

@mjackson mjackson reopened this Oct 12, 2018
@mjackson
Copy link
Member

I'm going to cherry-pick the <StrictMode> tests from this PR under a feature flag so we can make sure we don't print any errors in strict mode.

@mjackson
Copy link
Member

Sorry, I didn't mean to re-open. I'll go ahead and cherry-pick those tests myself.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants