immutable-js: hashing breaks for custom objects with "Invalid value used as weak map key"

I think the latest changes to Hash.js introduce a weird bug when hashing custom objects, more specifically, a class with valueOf() overridden. The error being thrown is Uncaught TypeError: Invalid value used as weak map key.

It’s strange because it doesn’t happen until a certain point, a very specific point. It happens when invoking .set() on a Map on the 9th attempt. For the first 8 attempts, all is good. On the 9th attempt, error.

To reproduce, I ran this bit of code in the console at https://facebook.github.io/immutable-js.

class Example {
  constructor(id, key) { this.id = id; this.key = key; }
  valueOf() { return `${this.id}.${this.key}`; } 
  toString() { return `${this.id}.${this.key}`; }
}

const m = Immutable.Map().asMutable();
m.set(new Example('test', (new Date()).toISOString()), Math.random()); // 1, ok
...
m.set(new Example('test', (new Date()).toISOString()), Math.random()); // 8, ok
m.set(new Example('test', (new Date()).toISOString()), Math.random()); // 9, ERROR

This has worked for me for quite a while. It breaks with release v4.0.0-rc.11.

Here’s a screenshot of the console:

screen shot 2018-11-06 at 12 17 27 pm

About this issue

Original URL
State: closed
Created 6 years ago
Reactions: 13
Comments: 22 (4 by maintainers)

Commits related to this issue

Fixes #1643 — committed to adamshone/immutable-js by deleted user 6 years ago
Fixes #1643 — committed to randytarampi/immutable-js by deleted user 6 years ago
fixed #1643 — committed to eGroupAI/immutable-js by abrcdf1023 4 years ago
Merge pull request #1 from eGroupTeam/issue/1643 fixed #1643 — committed to eGroupAI/immutable-js by abrcdf1023 4 years ago
fixed #1643 — committed to immutable-js-oss/immutable-js by abrcdf1023 4 years ago
fixed #1643 — committed to immutable-js-oss/immutable-js by abrcdf1023 4 years ago

Most upvoted comments

The magic number 9 is due to the MAX_ARRAY_MAP_SIZE check in ArrayMapNode.update where it uses a simple array for small numbers of keys, and only calls createNodes for larger numbers, so that is normal.

The bug itself seems to be in Hash.js from line 34:

if (o.valueOf !== defaultValueOf && typeof o.valueOf === 'function') {
    o = o.valueOf(o);
}
return hashJSObj(o);```

`hashJSObj(o)` here gets a string, but expects an object.
In the previous code the `o = valueOf(o)` assignment were performed first thing, and then treated as if it was the original input paramater.

joakim-hagglund on Nov 12, 2018

I’ve had the same/similar bug manifest in a different way.

When calling toSet() on a Map<string, T>, in some conditions (that, for the life of me I couldn’t figure out a small reproduction case for), I ran into the following error: TypeError: Attempted to set a non-object key in a WeakMap

It took me about 4 hours of debugging to figure out that it was occurring when hashCode() was being called on some of my Records. For some reason, calling toSet(), toSetSeq() (and maybe one or two more) threw exceptions… toList(), toMap(), toArray() all worked fine.

In any case, I merged @adamshone’s PR - and it fixed my issue too. Just wanted to post this to save someone some grief when Googling down the road.

sureshjoshi on Aug 19, 2019

We also encountered this issue and the fix in https://github.com/adamshone/immutable-js/commit/209817ed0cd584379824ba5f028e3baf7ab48b5d seems to solve. Any chance this can be merged?

kwek on Jun 13, 2019

I fixed it over a year ago but I think this project has been abandoned. We ran with a fork for a while and eventually switched to https://github.com/immerjs/immer

¯_(ツ)_/¯

adamshone on May 11, 2020

Using this as a workaround 🤦‍♂️

Date.prototype.hashCode = function () { 
	return Number(this);
};

futpib on Jul 9, 2019

I’ve incorporated this fix into https://github.com/immutable-js-oss/immutable-js/pull/17. We hope to be merging the community maintained fork back into immutable before we release. Otherwise we’ll release immutable-oss@4.0.0.

conartist6 on Aug 24, 2020

@conartist6 @lukas1994 - yes, immer has a dramatically different API. It’s focused only on immutability, basically, whereas Immutable is a full-featured data structure library.

Thanks for mentioning iter-tools. I haven’t seen your lib before, but I’ll check it out!

JoshuaToenyes on Jun 29, 2020

Gosh, that’s definitely worth a blog post!

Honestly, I’ve found immer a lot easier to work with, especially with Typescript (typing with Immutable, specifically Records got really tricking in some cases). I don’t have a list of specific things to watch out for off the top of my head. But if you’re interested in reading up on a larger project that refactored from Immutable to immer, checkout out Slate: https://github.com/ianstormtaylor/slate/issues/2190, https://github.com/ianstormtaylor/slate/issues/2345, and their milestone for tracking the process https://github.com/ianstormtaylor/slate/milestone/3.

JoshuaToenyes on Jun 29, 2020

o_O

conartist6 on Nov 9, 2018