search

browserify / common-shakeify

browserify tree shaking plugin using `common-shake`
https://www.npmjs.com/package/common-shakeify
MIT License
105 stars 18 forks source link

common-shakeify@0.6.1 crashes with browserify --full-paths option #32

Closed feross closed 5 years ago

feross commented 5 years ago

Issue did not exist in common-shakeify@0.6.0.

In common-shakeify@0.6.1, I started getting this error:

/Users/feross/code/bitmidi.com/node_modules/common-shakeify/index.js:92
    Object.keys(row.indexDeps).forEach((name) => {
           ^

TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at DestroyableTransform.onfile [as _transform] (/Users/feross/code/bitmidi.com/node_modules/common-shakeify/index.js:92:12)
    at DestroyableTransform.Transform._read (/Users/feross/code/bitmidi.com/node_modules/readable-stream/lib/_stream_transform.js:184:10)
    at DestroyableTransform.Transform._write (/Users/feross/code/bitmidi.com/node_modules/readable-stream/lib/_stream_transform.js:172:83)
    at Labeled.Pipeline._write (/Users/feross/code/bitmidi.com/node_modules/stream-splicer/index.js:70:22)
    at doWrite (/Users/feross/code/bitmidi.com/node_modules/readable-stream/lib/_stream_writable.js:428:64)
    at writeOrBuffer (/Users/feross/code/bitmidi.com/node_modules/readable-stream/lib/_stream_writable.js:417:5)
    at Labeled.Writable.write (/Users/feross/code/bitmidi.com/node_modules/readable-stream/lib/_stream_writable.js:334:11)
    at Labeled.ondata (/Users/feross/code/bitmidi.com/node_modules/readable-stream/lib/_stream_readable.js:619:20)
    at Labeled.emit (events.js:198:13)
/Users/feross/code/bitmidi.com/node_modules/disc/bin/discify:59
    if (err) throw err
             ^

Looking at the changes made for 0.6.1 this definitely seems like it was introduced there. Haven't had a chance to look further into the cause.

The full command I'm using is browserify --no-detect-globals --extension mjs --plugin tinyify --full-paths . with latest browserify.

goto-bus-stop commented 5 years ago

Crap, now I remember why I didn't use row.indexDeps in the first place :woman_facepalming: 0.6.1 switched from using row.deps to row.indexDeps which is more consistent when using the -r flag. thanks for the report. I think with fullPaths we can actually use row.deps, so could toggle which property it uses depending on whether that is enabled.

goto-bus-stop commented 5 years ago

:package: 0.6.2 disables the bugfix from 0.6.1 if you use --full-paths

feross commented 5 years ago

Thanks for the quick fix!