Ncc: Package `ws` doesn't include `bufferutil`

[NathanielHill]

ncc no longer includes bufferUtil package when compiling, this is a regression (although maybe in ws not ncc) as I haven't changed my lambda code since before this failure began occuring.

Here's the build messages:

running user script...
compiling entrypoint with ncc...
ncc: Version 0.18.5
ncc: Compiling file index.js
ncc: Module directory ".../jsdom/node_modules/ws/lib" attempted to require "utf-8-validate" but could not be resolved, assuming external.
ncc: Module directory ".../jsdom/node_modules/ws/lib" attempted to require "bufferutil" but could not be resolved, assuming external.
ncc: Module directory ".../node_modules/ws/lib" attempted to require "bufferutil" but could not be resolved, assuming external.
ncc: Module directory ".../node_modules/ws/lib" attempted to require "utf-8-validate" but could not be resolved, assuming external.
running builder.exports.prepareCache...

And this results in the following error:

Internal Server Error:  TypeError: bu.mask is not a function
    at mask (/var/task/api/report/index.js:103386:15)
    at Function.frame (/var/task/api/report/index.js:46990:5)
    at Sender.send (/var/task/api/report/index.js:47174:16)
    at WebSocket.send (/var/task/api/report/index.js:155580:18)
    at WebSocketTransport.send (/var/task/api/report/index.js:258703:14)
    at Connection._rawSend (/var/task/api/report/index.js:127113:21)
    at Connection.send (/var/task/api/report/index.js:127099:21)
    at Function.create (/var/task/api/report/index.js:63947:22)
    at Launcher.launch (/var/task/api/report/index.js:154229:37)
    at <anonymous>

[NathanielHill]

So, this looks like another "optional" dependency issue a la #274.

Looks like these are imported conditionally if they exist, but ncc is incorrectly bundling and running the try-catch block.

Here's the relevant ncc build output:

try {
  const bufferUtil = __webpack_require__(757);
  const bu = bufferUtil.BufferUtil || bufferUtil;

  module.exports = {
    mask(source, mask, output, offset, length) {
      if (length < 48) _mask(source, mask, output, offset, length);
      else bu.mask(source, mask, output, offset, length);
    },
    unmask(buffer, mask) {
      if (buffer.length < 32) _unmask(buffer, mask);
      else bu.unmask(buffer, mask);
    },
    concat
  };
} catch (e) /* istanbul ignore next */ {
  module.exports = { concat, mask: _mask, unmask: _unmask };
}

And this appears to be the source:
https://github.com/websockets/ws/blob/911bb6fb1145c9b09ae4b1cf4c993b3b8ce9e1cf/lib/buffer-util.js#L119-L144

This is related to:
https://github.com/GoogleChrome/puppeteer/issues/1242
https://github.com/GoogleChrome/puppeteer/issues/1224

https://github.com/websockets/ws/issues/1427
https://github.com/websockets/ws/issues/659
https://github.com/websockets/ws/pull/1311
https://github.com/websockets/ws/issues/1126
https://github.com/websockets/ws/issues/1040
https://github.com/websockets/ws/issues/1020
https://github.com/websockets/ws/issues/1010
https://github.com/websockets/ws/pull/740
https://github.com/websockets/ws/issues/719

[NathanielHill]

@styfle @guybedford

[NathanielHill]

I've temporarily resolved this by adding bufferutil and utf-8-validate to the lambda's dependencies.

[rauchg]

Thanks for the great description @NathanielHill

[NathanielHill]

@rauchg This was hard to track down, and ultimately was the same root cause as a problem I had with jsdom. Wondering if you guys have any magic planned to solve the optional/conditional dependency issues when compiling javascript? Seems like unfortunate growing pains for a new paradigm.

[styfle]

Hi Nathaniel,

The issue you referenced, #274, seems to be the opposite problem.
You were expecting the optional dependency to be discarded.
My comment shows a test that proved the optional dependency was not included as expected.

In this case, you are suggestion that the optional dependency should be included.

Upon further investigation, the ws package is using a conditional require to detect if a particular devDependency is available (it won't be unless running tests).

The workaround, as you mentioned above, is to add the optional dependencies to your package.json.

However, I am not able to reproduce this issue.

yarn init -y
yarn add ws
echo "let ws = require('ws')" > index.js
ncc build index.js
node dist

A simple require doesn't produce the error.
Perhaps you could explain the steps to reproduce this issue?

[NathanielHill]

@styfle Yes, I suppose they are opposite sides of the same underlying problem. I haven't gotten back around to #274, but I imagine it is resolved now. I was using a custom jsdom fork to resolve the issue, but I will try again.

I think the problem here is that the conditional require is being run, and the bufferutil import is failing silently, so we end up getting a reference error later. The import should throw an exception so the try block fails and we execute the fallback logic.

I can't share the code, so not much help if you can't reproduce. May learn more when I get to resolving #274

[Djiit]

Hey :wave: , is there any workaround for this ? Thanks

[NathanielHill]

@Djiit You should be able to solve it by including the optional dependency.

I'm unable to reproduce this, so closing for now.

Feel free to open a new issue if you're experiencing a similar problem.

@styfle

[Djiit]

Oh yes, that did the trick. Should we write down something in the readme about conditional requires and maybe take ws as an example? I can make a PR in the morning.