Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile with warnings #1220

Closed
albandaft opened this issue Oct 20, 2017 · 34 comments
Closed

Compile with warnings #1220

albandaft opened this issue Oct 20, 2017 · 34 comments

Comments

@albandaft
Copy link

albandaft commented Oct 20, 2017

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

This issue is coming from the zalmoxisus/remote-redux-devtools#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
Copy link
Member

lpinca commented Oct 20, 2017

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
Copy link
Member

lpinca commented Oct 21, 2017

@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
Copy link
Member

lpinca commented Oct 21, 2017

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

@lpinca lpinca closed this as completed Oct 22, 2017
@albandaft
Copy link
Author

@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
Copy link
Member

lpinca commented Oct 23, 2017

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
Copy link
Author

albandaft commented Oct 23, 2017

@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
Copy link
Member

lpinca commented Oct 23, 2017

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
Copy link
Author

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
Copy link
Member

lpinca commented Oct 23, 2017

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
Copy link
Author

None of the above worked.

@lpinca
Copy link
Member

lpinca commented Oct 26, 2017

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 webpack@3.8.1.

@albandaft
Copy link
Author

@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
Copy link
Member

lpinca commented Oct 26, 2017

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

@waheed25
Copy link

waheed25 commented Nov 13, 2017

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

@syang
Copy link

syang commented Oct 2, 2018

Is the warning negligible? Or should we install bufferutil ?

@gosp
Copy link

gosp commented Oct 15, 2018

@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
Copy link

KATT commented Oct 25, 2018

Or:

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

@u-bits
Copy link

u-bits commented Jan 28, 2019

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

MoOx added a commit to MoOx/phenomic that referenced this issue Mar 15, 2019
WARNING in /Users/moox/Sync/Development/phenomic/node_modules/ws/lib/BufferUtil.js
Module not found: Error: Can't resolve 'bufferutil' in '/Users/moox/Sync/Development/phenomic/node_modules/ws/lib'

WARNING in /Users/moox/Sync/Development/phenomic/node_modules/ws/lib/Validation.js
Module not found: Error: Can't resolve 'utf-8-validate' in '/Users/moox/Sync/Development/phenomic/node_modules/ws/lib'

Ref websockets/ws#1220
@ozyman42
Copy link

ozyman42 commented Apr 27, 2019

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.

@ozyman42
Copy link

ozyman42 commented Apr 27, 2019

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
Copy link
Member

lpinca commented Apr 28, 2019

@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.

@ozyman42
Copy link

ozyman42 commented Apr 28, 2019

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
Copy link
Member

lpinca commented Apr 28, 2019

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?

@ozyman42
Copy link

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

@lpinca
Copy link
Member

lpinca commented Apr 28, 2019

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.

@ozyman42
Copy link

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
Copy link
Member

lpinca commented Apr 28, 2019

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

Yes, exactly. See 49b1109. 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?

@ozyman42
Copy link

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
Copy link
Member

lpinca commented Apr 29, 2019

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.

@ozyman42
Copy link

ozyman42 commented May 1, 2019

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
Copy link

etienne-martin commented Dec 9, 2019

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.

@ozyman42
Copy link

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.

@ozyman42
Copy link

@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
Copy link

@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

romandev0318 pushed a commit to romandev0318/phenomic that referenced this issue Dec 20, 2023
WARNING in /Users/moox/Sync/Development/phenomic/node_modules/ws/lib/BufferUtil.js
Module not found: Error: Can't resolve 'bufferutil' in '/Users/moox/Sync/Development/phenomic/node_modules/ws/lib'

WARNING in /Users/moox/Sync/Development/phenomic/node_modules/ws/lib/Validation.js
Module not found: Error: Can't resolve 'utf-8-validate' in '/Users/moox/Sync/Development/phenomic/node_modules/ws/lib'

Ref websockets/ws#1220
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants