Ws: Compile with warnings

[albandaft]

Hi guys
I have integrated the tool inside my project and I get this warning while compiling:

This issue is coming from the https://github.com/zalmoxisus/remote-redux-devtools/issues/102, but is a WS library issue.
Basically I notice that the BufferUtil.js check if the library is already present to use it, other use it implements it. The problem is that this will cause require to fire an warning.

WARNING in ./node_modules/ws/lib/BufferUtil.js
Module not found: Error: Can't resolve 'bufferutil' in '/Users/alban/projects/frontend-base/node_modules/ws/lib'
 @ ./node_modules/ws/lib/BufferUtil.js 35:21-42
 @ ./node_modules/ws/lib/Receiver.js
 @ ./node_modules/ws/index.js
 @ ./node_modules/socketcluster-client/lib/sctransport.js
 @ ./node_modules/socketcluster-client/lib/scsocket.js
 @ ./node_modules/socketcluster-client/index.js
 @ ./node_modules/remote-redux-devtools/lib/devTools.js
 @ ./node_modules/remote-redux-devtools/lib/index.js
 @ ./src/store/configureStore.js
 @ ./src/index.server.js
 @ multi ./src/index.server.js

WARNING in ./node_modules/ws/lib/Validation.js
Module not found: Error: Can't resolve 'utf-8-validate' in '/Users/alban/projects/frontend-base/node_modules/ws/lib'
 @ ./node_modules/ws/lib/Validation.js 10:22-47
 @ ./node_modules/ws/lib/Receiver.js
 @ ./node_modules/ws/index.js
 @ ./node_modules/socketcluster-client/lib/sctransport.js
 @ ./node_modules/socketcluster-client/lib/scsocket.js
 @ ./node_modules/socketcluster-client/index.js
 @ ./node_modules/remote-redux-devtools/lib/devTools.js
 @ ./node_modules/remote-redux-devtools/lib/index.js
 @ ./src/store/configureStore.js
 @ ./src/index.server.js
 @ multi ./src/index.server.js

Wouldn't it be better to just include these libraries in the package.json or remove totally them from loading?

[lpinca]

This is done on purpose as both bufferutil and utf-8-validate have a native component which can fail to compile. Originally they were included in package.json but they have been removed to prevent installation from failing when compilation was not successful.
Removing them completely is not an option as they greatly improve the library performances.

[lpinca]

@albandaft one way to get rid of the warnings is to exclude bufferutil and utf-8-validate when bundling.

In the browser you must use the native WebSocket object, not ws.

[lpinca]

To be honest I'm not even sure why you get the warnings as it should use the shimmed version provided by socketcluster-client: https://github.com/SocketCluster/socketcluster-client/blob/87ac534521471e8da6cc95967a7af059601fdd2a/package.json#L30-L32

[albandaft]

@lpinca The reason why the warnings appear is simple: To test if bufferutil or utf-8-validate is installed you just call require('bufferutil') which will through a warning if the library is not there.

How to exclude the bufferutil from building?

[lpinca]

remote-redux-devtools uses socketcluster-client which in turns uses ws. socketcluster-client uses this shim for the browser when it is bundled so you shouldn't get any warning.

[albandaft]

@lpinca I get this warning during the react command line building, not in the browser. It is not a matter of shim, it is simply a matter of require, which is used to test the presence of the library:

From https://github.com/websockets/ws/blob/master/lib/BufferUtil.js#L35

try {
  const bufferUtil = require('bufferutil');

  module.exports = Object.assign({ concat }, bufferUtil.BufferUtil || bufferUtil);
} catch (e) /* istanbul ignore next */ {
...
}

[lpinca]

How to exclude the bufferutil from building?

In Browserify you can use the --exclude option or Browserify#exclude(). Other bundlers have similar options/plugins.

I get this warning during the react command line building, not in the browser

Yes but I guess you are creating a bundle for the browser right?

[albandaft]

On the browser everything works fine. I need to remove the warnings because eventually a pipeline build process will fail to pass test if the build exit code is not correct.

I am actually running the bundle in memory in watch mode, dev mode.
I will try to workaround to exclude them from the building.

[lpinca]

It is not a matter of shim, it is simply a matter of require, which is used to test the presence of the library

try {
  const bufferUtil = require('bufferutil');

  module.exports = Object.assign({ concat }, bufferUtil.BufferUtil || bufferUtil);
} catch (e) /* istanbul ignore next */ {
...
}

That code along with all ws code should be completely ignored and replaced by the bundler with the shim code when instructed to do that.

The browser field is where the module author can hint to the bundler which elements (other modules or source files) need to be replaced when packaging.

[albandaft]

None of the above worked.

[lpinca]

index.js

'use strict';

module.exports = require('remote-redux-devtools');

webpack.config.js

'use strict';

module.exports = {
  entry: './index.js',
  output: {
    libraryTarget: 'umd',
    filename: 'bundle.js',
    library: 'enhancer'
  }
};

---

This doesn't generate any warning with [email protected].

[albandaft]

@lpinca With your example I do not get either the warnings. Here we get those warning in different machines. We will keep trying to understand...

[lpinca]

Are you requiring ws in other places or is remote-redux-devtools the only module that uses it via socketcluster-client?

[waheed25]

is there any solution to remove these warnings???
because none of the above works.

[syang]

Is the warning negligible? Or should we install bufferutil?

[gosp]

@syang
webpack.config.js :: externals: [/node_modules/, 'bufferutil', 'utf-8-validate'],

for example:

module.exports = {
  entry: { server: path.join(__dirname, 'src', 'script', 'index.ts') },
  resolve: { extensions: ['.js', '.ts'] },
  target: 'node',
  mode: 'none',
  // this makes sure we include node_modules and other 3rd party libraries
  externals: [/node_modules/, 'bufferutil', 'utf-8-validate'],
  output: {
    path: path.join(__dirname, '..', 'dist'),
    filename: 'server.js'
  },
  module: {
    rules: [{ test: /\.ts$/, loader: 'ts-loader' }]
  },
  plugins: []
};

[KATT]

Or:

module.exports = {
  // [..]
  externals: {
    bufferutil: 'commonjs bufferutil',
    'utf-8-validate': 'commonjs utf-8-validate',
  },
  // [..]
};

[u-bits]

I solved this warning by just changing the target in my webpack.config.js to web
https://webpack.js.org/configuration/target/

[AlexLeung]

Seems a bit weird that if I want to bundle socket.io-client via webpack then I need to explicitly externalize the bufferutil and utf-8-validate packages in my webpack config because of this flaw in the transitive dependency ws. Either socket.io-client should stop using ws, or ws should find/invent alternatives to bufferutil and utf-8-validate that it would be comfortable using. Those try catch require blocks in lib/buffer-util.js and lib/validation.js seem like an undesirable solution to the problem.

[AlexLeung]

Removing them completely is not an option as they greatly improve the library performances.

@lpinca I don't understand why that is the case. If the two packages are listed in package.json under devDependencies, then clients of the ws package will never download bufferutil and utf-8-validate, and thus will never experience the performance increase you speak of.

[lpinca]

@AlexLeung if you want to use bufferutil utf-8-validate (recommended), you have to explicitly add them to your dependencies:

npm install ws bufferutil utf-8-validate

or

npm install ws
npm install --save-optional bufferutil utf-8-validate

ws will use them when available.

[AlexLeung]

Why aren't bufferutil and utf-8-validate saved as optionalDependencies in this repo?
I think that would at least make more sense than devDependencies, and wouldn't require a code change besides to package.json, then webpack core could be updated to know not to throw warnings when transitive dependencies' optionalDependencies are not installed, and just auto externalize them.

[lpinca]

They are dev dependencies because tests must pass with them. If we move them to optional dependencies they are no longer opt-in.

Out of curiosity, are you making a bundle for the browser or Node.js?

[AlexLeung]

I thought the definition of something being optional is that it is opt-in.
I am creating a bundle for Node.js

[lpinca]

I thought the definition of something being optional is that it is opt-in.

Not for npm, you have to use the --no-optional flag to not install optional dependencies.

[AlexLeung]

npm will try and install those optional deps, but if the install fails for those, it wouldn't fail the overall install, which is what you were originally trying to do.

they have been removed to prevent installation from failing when compilation was not successful.

[lpinca]

they have been removed to prevent installation from failing when compilation was not successful.

Yes, exactly. See https://github.com/websockets/ws/commit/49b11093e9a009e5305dcde7003d3a896b2811dc. This was done because install failed even though they were optional dependencies.

Now npm has probably fixed the the issue but I see no good reasons to add them back. What's wrong with externalizing them in webpack config?

[AlexLeung]

I see. Failing in optional dependencies should not fail the entire installation, but if it was still failing in this case I can understand why it was placed in devdeps.

Externalizing these in the webpack config is undesirable because the point of a package/library is to abstract away / encapsulate behavior so you don't need to know how it works, you just use it. Needing to externalize these couple of packages because ws is a transitive dependency of many other libraries (some of which end up being bundled), seems to break the encapsulation that a package typically would provide. Worse it breaks this encapsulation all the way up the dependency tree such that no package which depends on ws can hide this from its own clients.

Maybe we could reopen this issue and mark as in-progess until utf-8-validate and bufferutil are replaced, or until fixes are made to those so they don't fail.

[lpinca]

Nothing breaks, the warnings are just that and can be safely ignored.

The same happen if you try to bundle a library which uses external dependencies in the same way ws does so in my opinion it's more an "issue" with bundlers than ws.

[AlexLeung]

Yes, exactly. See 49b1109. This was done because install failed even though they were optional dependencies.

This sounds like a bug within npm.

[etienne-martin]

For me the issue was that I had target: "node" in my webpack.config.js while I was trying to bundle for the web.

Switching to target: "web" removed the warnings.

[AlexLeung]

Now npm has probably fixed the the issue but I see no good reasons to add them back [to optionalDependencies]. What's wrong with externalizing them in webpack config?

Nothing breaks, the warnings are just that and can be safely ignored.

In general it's bad practice for a library to cause warnings if there is no actual problem, it clutters the console and wastes everyone's time searching for the root cause of a non-issue. This is good enough reason to add back to optionalDependencies.

[AlexLeung]

@etienne-martin I'm surprised that you saw the issue at all since #1626 should have fixed the problem. I'd be cautious of switching target to web, because that could cause greater issues if your bundle really is supposed to target node.

[Dahkenangnon]

@AlexLeung if you want to use bufferutil utf-8-validate (recommended), you have to explicitly add them to your dependencies:

npm install ws bufferutil utf-8-validate

or

npm install ws
npm install --save-optional bufferutil utf-8-validate

ws will use them when available.

Using this, i solve the warning