hashCode attempts to modify frozen object

[Methuselah96]

From @bdurrer on Thu, 13 Jun 2019 17:44:19 GMT

I already wrote this into an old issue (immutable-js#1228), which was about an older release version and is closed.

What happened

When working with records in collections, Immutable sometimes tries to set an hashCode of a frozen object, resulting in an exception.
Expected behavior: No exception, of course. I expect hashCode to be set when creating an instance, not generated later on.

The hashCode function looks like a getter but apparently it sets it too:

  hashCode: function hashCode() {
    return this.__hash || (this.__hash = hashCollection(this));
  },

How to reproduce

It started happening after upgrading from 4.0.0-rc.9 to 4.0.0-rc.12.
I am very sorry, but I was unable to create an example which triggers this behavior. It only occurs in a big react/redux/saga project, where the redux state is using Immutable collections and various Record types heavily.

The behavior can be reproduced in said project, it reliably appears in the same code location. However, it appears in weird places, e.g. at one point a call to orderedMap.has(itemRecord) throws it, somewhere else it is a list.valueSeq() that fails.
Furthermore, calling orderedMap.has(itemRecord) in some other location might not produce an error at all. It seems like it is an arcane order of things that need to happen, before the error occurs.

Anyway, here is an stacktrace, which was triggered by a seemingly simple orderedMap.has(itemRecord):

In this case, has() triggers hashCode() on the Record, which is already part of the orderedMap and (in my understanding) should already have set the hash code.

immutable.es.js:5032 Uncaught TypeError: Cannot assign to read only property '__hash' of object '[object Object]'
    at List.hashCode (immutable.es.js:5032)
    at hash (immutable.es.js:775)
    at immutable.es.js:5310
    at immutable.es.js:1111
    at ArraySeq.__iterate (immutable.es.js:452)
    at FromEntriesSequence.__iterate (immutable.es.js:1105)
    at hashCollection (immutable.es.js:5303)
    at FromEntriesSequence.hashCode (immutable.es.js:5032)
    at Record.hashCode (immutable.es.js:5479)
    at hash (immutable.es.js:775)

Originally posted by @bdurrer in immutable-js#1228 (comment)

Copied from original issue: immutable-js#1717

[bdurrer]

I found the solution to it in immutable-js#1717
redux-freeze was freezing my Immutables, while _hashCode is lazily initialized.
So this was only a problem during development, because that is where you use redux-freeze. Patching redux-freeze fixed the problem (for me).

[Methuselah96]

@bdurrer Yeah, still wasn't sure whether to keep this open or not, because it seemed like a change in behavior from immutable@3 to immutable@4 and I wasn't sure if there was a conclusion whether it should be fixed by immutable or redux-freeze.

[bdurrer]

It is a regression, yes. I am also not sure if we should care about it tho.
I assume the change was done for performance reasons, since the hashCode seems to only be relevant in Records or properties of Records, so no need to calculate it before it is required

[Methuselah96]

@conartist6 Any opinion on this one?

[Methuselah96]

After reviewing this, I don't think this is really immutable's concern. The code internally sets properties on the Immutable objects quite frequently and it's up to other libraries to figure out how to work with Immutable on this one.

[conartist6]

This isn't about setting properties on an immutable object, it's about setting properties on any object used as a key. The real fix is to drop IE11 support and use Maps internally instead of this bonkers system with objects and hash codes.

[Methuselah96]

Okay, interesting, I didn't look at it closely enough. Do you think this is worth changing for 4.0?

[conartist6]

I guess I think it depends. We could always push it to 5.0. And it'd be a fair amount of work I think. Who is going to do it? We'd probably want to rip out hashCode and hash and valueObject too, since those are all part of this crazy solution to make up for the lack of Maps.

[bdurrer]

I would postpone it or even make redux-freeze fix it instead

[Methuselah96]

I was able to reproduce this in 3.8.2 (CodeSandbox) so I'm removing it from the 4.0 milestone. (Perhaps there was a change that made it happen more often in some of the 4.x releases?)

[bdurrer]

I think it usually happend when modifying Records in Maps, but I never figured out how to pinpoint it, it felt random. Oops , misclicked on close