Skip to content

hash no longer works with moments #1638

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.

Sign up for GitHub

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

Jump to bottom
Closed
corcorb opened this issue Oct 31, 2018 · 5 comments
Closed

hash no longer works with moments #1638

corcorb opened this issue Oct 31, 2018 · 5 comments
Labels
bug
Milestone
4.0

Comments

@corcorb
Copy link

corcorb commented Oct 31, 2018

What happened

As of 4.0.0-rc.11 the hash function no longer works with moment values.

It gives an error of:

/test/immutablejs-test/node_modules/immutable/dist/immutable.js:882
      weakMap.set(obj, hashed);
              ^

TypeError: Invalid value used as weak map key
    at WeakMap.set (<anonymous>)
    at hashJSObj (/test/immutablejs-test/node_modules/immutable/dist/immutable.js:882:15)
    at hash (/test/immutablejs-test/node_modules/immutable/dist/immutable.js:794:16)
    at Object.<anonymous> (/test/immutablejs-test/index.js:6:14)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:742:12)

How to reproduce

Run the following program with immutable:4.0.0-rc.11, moment:2.22.2, node:v9.11.2:

const { hash } = require('immutable');
const moment = require('moment');

const now = moment();

const res1 = hash(now)
console.log('hash of moment', res1);

Above works on 4.0.0-rc.10, but fails on rc.11 and rc.12.

FWIW, I can work around this on my side and it isn't a good practice to hash a mutable variable anyway. But I wanted to report it in case it's a symptom of a bigger issue.
@ryansname
Copy link

ryansname commented Nov 1, 2018 •
edited
Loading

I also found a similar problem, with the same error, minimal example works on the Immutable.js webpage.

Immutable.Record({a: new Date()})().hashCode().

Without going into it too deeply the data changes here from an object to a number, which then causes problems in hashJSObj as we're no longer passing it an object because of the valueOf

Minimal test in __tests__/hash.ts:

  it('can hash a date', () => {
    hash(new Date(0));
  });

@farnoy
Copy link

farnoy commented Nov 2, 2018

I worked around it by defining the method hashCode on moment prototype to use valueOf:

farnoy/frozen-moment@8a5015b

Works fine so far

@adamshone
Copy link

I think this is a dupe of #1643 (or that one is a dupe of this)

@Methuselah96 Methuselah96 mentioned this issue Oct 17, 2020
Closed
@Methuselah96 Methuselah96 added this to the 4.0 milestone Jun 18, 2021
@jdeniau
Copy link
Member

jdeniau commented Jun 29, 2021

Thank you for your bug report. The immutable-js oss fork will soon be merged and a fix for this issue has been included in #1833. Once this PR is merged, this issue will be resolved in the main branch. We will then do our best to to release the 4.0.0 version.

Commit reference: 45196e3

@jdeniau
Copy link
Member

jdeniau commented Jul 8, 2021

Hi!
The 4.0.0-rc.14 version has been released and should fix this issue.
Feel free to test it and re-open this issue if the issue is still present.
Thank you!

@jdeniau jdeniau closed this as completed Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
4.0
Development

No branches or pull requests
6 participants
@farnoy @ryansname @Methuselah96 @adamshone @jdeniau @corcorb