Skip to content
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

Using lodash isPlainObject on a union instance throws an exception #190

Closed
ohana54 opened this issue Apr 30, 2018 · 2 comments
Closed

Using lodash isPlainObject on a union instance throws an exception #190

ohana54 opened this issue Apr 30, 2018 · 2 comments

Comments

@ohana54
Copy link

ohana54 commented Apr 30, 2018

Hi,
We encountered an issue with Union and method definitions, where running lodash.isPlainObject throws an exception on a union type.
When declaring a union, we use the ES6 shorthand method declaration:

const TestUnion = union('TestUnion', {
    Nothing() { }
})

This means that TestUnion.Nothing does not have a constructor in the same way that a "regular" function has, which fails the following check in lodash: https://github.com/lodash/lodash/blob/4.17.10/lodash.js#L12044
This is according to spec, and is also described here, under the title "Method definitions are not constructable".

Note: this does not happen with the built-in union Maybe.

Our use case:
We use lodash.merge function to gather application state from several sources into one object. Some of the state includes union instances. lodash.merge uses isPlainObject internally, thus failing the merge.

Steps to reproduce

This is the piece of code that reproduces the problem. There's also a runnable version here:
https://runkit.com/ohana54/folktale-lodash-bug

const union = require('folktale/adt/union/union')
const isPlainObject = require('lodash/isPlainObject')

const TestUnion = union('TestUnion', {
    Working: function () { },
    NotWorking() { }  
})

// Works
isPlainObject(TestUnion.Working())

// Throws "TypeError: Function has non-object prototype 'undefined' in instanceof check"
isPlainObject(TestUnion.NotWorking())

Expected behaviour

Expected isPlainObject not to throw an error

Observed behaviour

isPlainObject Throws TypeError: Function has non-object prototype 'undefined' in instanceof check

Environment

  • Mac OSX
  • Node 8, Node 9
  • Folktale v2.1.0

Thank you for this awesome library, we appreciate your time creating and maintaining it :)

@robotlolita
Copy link
Member

The fix has been merged on master. This will probably be a major release though. It's unlikely to break things, but previously the initialiser (NotWorking in this case) was exposed as the constructor for that variant. This was wrong, but there may be people relying on this bug so a major version would be the safe thing to do :/

@robotlolita robotlolita added this to the 4.0.0 milestone Jul 1, 2018
@ohana54
Copy link
Author

ohana54 commented Jul 2, 2018

@robotlolita that's great, thanks!

@ohana54 ohana54 closed this as completed Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants