React-helmet: deep-equal crashes on newer React?

Created on 25 Feb 2019  ·  22Comments  ·  Source: nfl/react-helmet

I've experienced some call-too-deep errors, and I think somehow React is introducing circular references and then deep-equal follows them.

I worked around this like so:

import {isEqual} from 'lodash'

// Helmet uses deep-equal and sometimes crashes on circular objects
Helmet.prototype.shouldComponentUpdate = function(nextProps) {
    return !isEqual(this.props, nextProps)
}

It seems to work - lodash has circular reference protection.

Most helpful comment

@tmbtech I can see the fix has now been pushed to a branch - great! Is 5.2.1 going to be published to npm soon?

All 22 comments

I got the same error on Firefox (InternalError: too much recursion) with React 16.8.3 and Helmet 5.2.0.

Thanks for your code @wmertens, I'll test it :)

Ugh.. just spent 2+ hours on top of this.
They seem to have replaced deep-equal with react-fast-compare on sCU, which should fix it. But no version was published under v5 since a LONG time ago.

If any maintainer could check the possibility of publishing a minor/patch on top of that commit https://github.com/nfl/react-helmet/commit/1803f678b9d38b5ae91961ea13ea1c7fa7c9afe8

This issue seems to be related https://github.com/nfl/react-helmet/issues/437

Have you tried to use v.6-beta?
More info here - https://github.com/nfl/react-helmet/pull/421#issuecomment-450449810

Yesterday I quickly tried it but didn't notice the import API change so most of my tests failed, I was too annoyed to debug that.

Now after reading that PR you linked it was pretty straightforward, already in prod.
Many thanks

I have to say though, it would be nice to keep v5 working, specially if it's a simple fix (publish that commit). Thanks again.
Edit: And also because v6 is still beta.

I'm no longer working at the NFL. Someone on the NFL frontend team would need to do it.

Yeah we ran into the same maximum call stack error issue with the latest CRA prod build. We ended up just removing react-helmet for now unfortunately. Upgrading to 6.0.0-beta didn't fix the issue for us.

I can confirm it's unusable with React 16.8.x, it crashes after a few renders
This fix is in master but not in 5.0.2. It has been fixed in November last year but there is still no release of the 5.x.
Until this gets released, I've published my own package rl-react-helmet which is just 5.0.2 + this patch.

@cwelch5 - any chance you can give this issue some love? It looks like a quick release of this package to NPM would help lots of us out 🙏

Finally I've found this issue!
I been spending 4 hours now debugging and now I've finally found the root cause. I would really appreciate an update to this. Thank you!

@janhartmann it's a somewhat unsatisfying solution, but we got around this (for the time being) by using [email protected] in our package.json.

I don't normally do this, but in this case of a seemingly abandoned library
I'd like to point out that the fork react-helmet-async is only a small
change to implement, it has the fix, it is updated, and it is better for
SSR.

Thanks to the NFL in any case for the great library!

On Tue., Mar. 12, 2019, 12:41 p.m. Mike Bifulco notifications@github.com
wrote:

@janhartmann https://github.com/janhartmann it's a somewhat
unsatisfying solution, but we got around this (for the time being) by using
[email protected] in our package.json.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/nfl/react-helmet/issues/441#issuecomment-471965965,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADWll_sNiyIqna2nY8QvxbFWBp-BKNAks5vV5JagaJpZM4bPLdi
.

Cheers, thanks for the alternatives. I'll check it out!

Use props syntax instead of children html. It happened to me also when dispatching multiple redux actions (middlewares) on the same event (was trying to dispatch a set spinner action when fetching some data).

@tmbtech I can see the fix has now been pushed to a branch - great! Is 5.2.1 going to be published to npm soon?

I could really use this fix , so I hope this fix can be published quickly! 👍

Sorry I don't have publishing right. @miblanchard @cwelch5 do you have any time to publish to NPM?

@miblanchard was so kind and published an updated version.

Closing, thank you all.

Oh my goodness, thank you so much @tmbtech and @miblanchard!

@tmbtech @miblanchard Thanks!

I still get errors with 6.0.0 using Gatsby, but the original workaround works for me.

I am facing the issue even after using v5.2.1, but the above give workaround works.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

tiagonapoli picture tiagonapoli  ·  3Comments

michaelBenin picture michaelBenin  ·  3Comments

bratva picture bratva  ·  4Comments

mattecapu picture mattecapu  ·  4Comments

brianespinosa picture brianespinosa  ·  3Comments