search

browserify / browser-resolve

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

Fix Node v6 #80

Closed msokk closed 8 years ago

msokk commented 8 years ago

Latest Node is more strict with path.dirname, throwing on passing undefined - https://github.com/nodejs/node/pull/5348

Also added newer Node versions to Travis test matrix.

zhanzhenzhen commented 8 years ago

Do you mean the problem is in this package, not the resolve package?

I have opened an issue here but got no response: https://github.com/substack/node-resolve/issues/98

defunctzombie commented 8 years ago

If this doesn't get fixed upstream in resolve ping me in a few days and I'll release this patch.

zhanzhenzhen commented 8 years ago

@defunctzombie Sorry I just have time to test resolve. It works! So I think the problem is only in this package.

mattbailey commented 8 years ago

@defunctzombie doesn't look like this has been resolved upstream yet; could you get this patched in?

Thanks!

zhanzhenzhen commented 8 years ago

@defunctzombie It seems the bug is only in browser-resolve, not in the upstream resolve package through my test. So you don't have to wait.

msokk commented 8 years ago

Yes, the exception happens in this package, it has nothing to do with upstream.

Currently the https://github.com/rollup/rollup-plugin-node-resolve downstream package is broken due to this on Node v6.

zhanzhenzhen commented 8 years ago

why this is still unmerged? Tested Node 6.2 but the problem still exists.

nkoterba commented 8 years ago

👍 Please check this patch in. I just submitted another pull request (https://github.com/defunctzombie/node-browser-resolve/pull/81) to essentially fix the same problem. As @msokk mentioned, this is breaking https://github.com/rollup/rollup-plugin-node-resolve and our build system.

Thanks.