thlorenz / proxyquireify

browserify >= v2 version of proxyquire. Mocks out browserify's require to allow stubbing out dependencies while testing.
MIT License
152 stars 24 forks source link

getting "Property description must be an object" from 'merge-descriptors' module (dependency of 'fill-keys') #35

Closed bitwit closed 9 years ago

bitwit commented 9 years ago

I seem to be having an issue whenever I proxyquireify with the TypeError: Property description must be an object. getting thrown. It looks like its trying to merge everything together in the merge-descriptors sub-sub dependency on this line. So far I've tried with with both jquery and mithril npm modules with the same problem.

It seemed like this repo was the best place to report the problem or get feedback on why this might be happening.

Thanks

bendrucker commented 9 years ago

Honestly no ideas here. If you can get a repro project online I'll step through and see how a descriptor managed to end up as a non-object. Would also help to have confirmation on the browsers you've tried.

bitwit commented 9 years ago

Yep, just about to update this. I've isolated it to PhantomJS and will report back with more.

bitwit commented 9 years ago

To say the least, I don't consider it an issue of Proxyquireify any longer, though the problem might be relevant to other users of this package. Will try to come back with anything I learn.

bendrucker commented 9 years ago

Phantom is a menace

The tricky thing about Phantom is that it implements all kinds of things completely wrong. At least the missing fn.bind is truly missing. Other stuff is defined but totally broken.

If there's a way to fix this without special-casing Phantom I'm happy to do it.

bitwit commented 9 years ago

haha, very appropriate image

Here's a proof project : https://github.com/bitwit/proxyquireify-phantom-menace succeeds in chrome, fails in phantom

Also while googling my way around I stumbled on this: https://github.com/petkaantonov/bluebird/issues/556

Looks like Bluebird had a similar issue recently and I can confirm that from what I saw, it was also related to 'callee'.

Hopefully there's an easy fix but, if not, hopefully PhantomJS 2.0 addresses this natively.

Cheers

bendrucker commented 9 years ago

Phantom 1.9 continues to be the worst.

function fn () {}
Object.prototype.hasOwnProperty.call(fn, 'callee') === false
Object.getOwnPropertyNames(fn).indexOf('callee') !== -1

Those two equalities make no sense. And this is only wrong in 1.9.8, not 2.0. So I'll release a shim and send some PRs around to link it in docs.

Phantom 1 can't die fast enough!

bendrucker commented 9 years ago

https://github.com/bendrucker/phantom-ownpropertynames

bitwit commented 9 years ago

:+1: works like a charm

Thanks for your help and responsiveness

ptarjan commented 8 years ago

Any luck upstreaming this?

bendrucker commented 8 years ago

Upstreaming as in including in proxyquireify? That's not on the table. We consider Phantom 1 a broken browser that requires shimming, no different than a browser without ES5 support. Shimming will be left up to the user.

ptarjan commented 8 years ago

Sorry, upstreaming to PhantomJS so they are no longer a broken browser.

bendrucker commented 8 years ago

Ah no, it's massive project and I'm not interested enough to work on getting a true patch into 1.9.

ptarjan commented 8 years ago

Is there an issue opened against them at least so I can go push it?

This doesn't yield anything: https://github.com/ariya/phantomjs/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+getOwnPropertyNames