-
Notifications
You must be signed in to change notification settings - Fork 13
recognize iterable type by constructor reference for serialization #12
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.
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
recognize iterable type by constructor reference for serialization #12
Conversation
src/serialize.js
Outdated
| return 'Map' | ||
| } | ||
|
|
||
| if (iterable.constructor === immutable.OrderedMap) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nechceš to spíš udělat jako switch (iterable.constructor), aby to bylo kompaktnější?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jo sem kkt, switch tam dám.
| debug('replaceIterable()', iter) | ||
|
|
||
| const iterableType = iter.constructor.name | ||
| const iterableType = getIterableType(iter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To se klidně mohl jen upravit ten switch tady.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no já potřebuju ten string pro tu serializaci, takže jsem to udělal přes metodu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ach tak, jasně, jsem vůl. Ok.
src/serialize.js
Outdated
| return 'Map' | ||
| } | ||
|
|
||
| if (iterable.constructor === immutable.OrderedMap) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chtělo by to test na tu serializaci. Asi tak, že bys pro ten test monkey-patchnul immutable.Map.constructor.name, pokud to jde.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
na to tam testy jsou normálně
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Já vim, ale fixujem bug přece. :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jo takle myslíš test, jasně
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tak to nejde - cannot assign readonly property name
e62e0d8 to
2239268
Compare
closes #10