Uses legacy componentWillMount

[jfrolich]

This component does not work in concurrentMode

[lourd]

Hey Jaap, thanks for noticing and filing an issue!

I went ahead and started work at resolving this on the use-did-mount branch. Can you please review it and also add a test case that makes sure the library is now concurrent mode compliant? Just fork from my branch.

Let me know if you have any questions. Thanks!

[jfrolich]

Not sure I have time to write a test, I don't use this library directly, just wanted to report it :)

[tyscorp]

Changing from componentWillMount to componentDidMount isn't enough as described here: #40

[seansfkelley]

This also triggers warnings when using StrictMode.

[mikecousins]

This now barfs warnings in React v16.9.

[lourd]

I'm gonna ping Dan on Twitter and ask if he can chime in here.

[gaearon]

I’ll take a look on Monday if I don’t forget. However this whole library doesn’t really make sense to me anymore so I’d encourage migrating away from these patterns. For example it wouldn’t work with a streaming renderer. Something using context would probably make more sense.

[mikecousins]

Yeah, it's used in react-document-title. So I guess we'll need to get away from using that or rewrite react-document-title away from react-side-effect. react-helmet also uses it.

[danielweck]

Sorry, I posted my previous comment in the wrong repo! :)

[kud]

I'm here only because I also use react-document-title.

@mikecousins ' solution sounds the idea!

[verheyenkoen]

I see commit f75bc96 already added the UNSAFE_ prefix to the componentWillMount method to suppress the warning for this React major version.

Not sure if this makes any sense but I just locally replaced that method name with componentDidMount (as suggested by the warning message) and all tests still pass. Could that be enough to fix this issue as a whole?

[lourd]

Unfortunately not, I believe there's just a coverage gap in the tests regarding mounting order. There's discussion in another issue or PR where we figured that out.

…

On Tue, Sep 17, 2019, 9:26 AM Koen Verheyen ***@***.***> wrote: I see commit f75bc96 <f75bc96> already added the UNSAFE_ prefix to the componentWillMount method to suppress the warning for this React major version. Not sure if this makes any sense but I just locally replaced that method name with componentDidMount (as suggested by the warning message) and all tests still pass. Could that be enough to fix this issue as a whole? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#54?email_source=notifications&email_token=AAPLJZGUEHJBWFQCNANAVB3QKDLGRA5CNFSM4GDCIF22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD64QOBQ#issuecomment-532219654>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPLJZCADDBXAYRGASW2INLQKDLGRANCNFSM4GDCIF2Q> .

[codeaid]

So does this issue that was opened back in 2018 have any resolution? React helmet is throwing Using UNSAFE_componentWillMount in strict mode is not recommended and it seems it's because of this library.

[kud]

-> https://github.com/staylor/react-helmet-async