search

browserify / browser-resolve

resolve function which support the browser field in package.json
MIT License
102 stars 70 forks source link

Confirm key existence strictly #74

Closed hakatashi closed 8 years ago

hakatashi commented 9 years ago

I've experienced problem when I combined browserify and gulp-mocha tasks in my Gulp task. I use should.js for my mocha test cases and it breaks code with defiining Object.prototype.should.

Though I know extending native objects are bizarre, checking key existence without hasOwnProperty is enough bad. Please confirm my quick fix.

defunctzombie commented 8 years ago

Alternatively, I think a better fix would be to change this line to use Object.create(null) to avoid any prototype (and thus making it a hash map like object as we want).

https://github.com/hakatashi/node-browser-resolve/blob/patch-1/index.js#L80

See if that solves your problem and lets update the PR to do that instead since it better aligns with the intended use of the shims var.

hakatashi commented 8 years ago

It seems nice. I'll perform update soon.

hakatashi commented 8 years ago

@defunctzombie I've done editing PR. To clearify problem I also added a test case which might previously failed.

defunctzombie commented 8 years ago

Awesome. Please remove the first two commits since they are no longer needed.

hakatashi commented 8 years ago

done