search

glenjamin / transit-immutable-js

Transit serialisation for Immutable.js
MIT License
250 stars 31 forks source link

use "new Record" to ensure record works #12

Closed corbt closed 8 years ago

corbt commented 8 years ago

Immutable.js confusingly allows for two syntaxes to create a new Record object: MyRecord({ args }) and new MyRecord({ args }). However, only the latter syntax works when you've created a record by extending the Record class instead of just calling it (see docs).

This PR uses the new MyRecord syntax, which is compatible with both ways of creating Records.

Additionally, I rename Record to RecordConstructor in the rehydration step to make it more clear that we're not dealing with Immutable's Record at this point.

corbt commented 8 years ago

If this looks good I'd appreciate a new release with this bugfix. :+1:

glenjamin commented 8 years ago

It looks good, could you add a failing testcase for this so we're covered against regression?

Then I'll get a release pushed out.

corbt commented 8 years ago

Well the failing you'd normally run into would be with the class MyRecord extends Record() syntax, and we don't have ES6 in the tests... but I'll try to reverse the ES6 class implementation and make a test that will have equivalent semantics.

glenjamin commented 8 years ago 
function MyRecord(){}
MyRecord.prototype = Object.create(Record);

That should cover it I think.