Fix Node v6 #80
Fix Node v6 #80
Conversation
|
Do you mean the problem is in this package, not the I have opened an issue here but got no response: |
|
If this doesn't get fixed upstream in resolve ping me in a few days and I'll release this patch. |
|
@defunctzombie Sorry I just have time to test |
| @@ -2,3 +2,6 @@ language: node_js | |||
| node_js: | |||
| - "0.8" | |||
| - "0.10" | |||
| - "4" | |||
| - "5" | |||
| - "6" | |||
There was a problem hiding this comment.
Shouldn't this just be "node" so it's always latest stable?
There was a problem hiding this comment.
Depends how many Node versions we want to test against? 4 is still LTS.
|
@defunctzombie doesn't look like this has been resolved upstream yet; could you get this patched in? Thanks! |
|
@defunctzombie It seems the bug is only in |
|
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. |
|
why this is still unmerged? Tested Node 6.2 but the problem still exists. |
|
👍 Please check this patch in. I just submitted another pull request (#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. |
Please see: rollup#38 and browserify/browser-resolve#80 for how Node v6+ throws a TypeError if `path.dirname` is passed an undefined value. `node-browser-resolve` currently does not use set a default value if it's `opts.filename` is not specified resulting in the following stack trace: ```javascript [19:40:45] TypeError: Path must be a string. Received undefined at assertPath (path.js:7:11) at Object.dirname (path.js:1324:5) at resolve (/my_app/node_modules/browser-resolve/index.js:218:21) at /my_app/node_modules/rollup-plugin-node-resolve/dist/rollup-plugin-node-resolve.cjs.js:46:5 at resolveId (/my_app/node_modules/rollup-plugin-node-resolve/dist/rollup-plugin-node-resolve.cjs.js:45:11) at /my_app/node_modules/rollup/dist/rollup.js:2123:32 at tryCatch (/my_app/node_modules/rollup/dist/rollup.js:393:12) at invokeCallback (/my_app/node_modules/rollup/dist/rollup.js:405:13) at publish (/my_app/node_modules/rollup/dist/rollup.js:376:7) at flush (/my_app/node_modules/rollup/dist/rollup.js:120:5) ```
Latest Node is more strict with
path.dirname, throwing on passing undefined - nodejs/node#5348Also added newer Node versions to Travis test matrix.