dmy / elm-doc-preview

Elm offline documentation previewer
https://www.npmjs.com/package/elm-doc-preview
Other
128 stars 15 forks source link

npm complains about peer dependencies #35

Closed malaire closed 4 years ago

malaire commented 4 years ago

When running

$ npm install -g elm-doc-preview

npm complains:

npm WARN ws@7.2.3 requires a peer of bufferutil@^4.0.1 but none is installed. You must install peer dependencies yourself.
npm WARN ws@7.2.3 requires a peer of utf-8-validate@^5.0.2 but none is installed. You must install peer dependencies yourself.

I then installed those two, but even after that I still get same warnings:

$ npm install -g bufferutil@^4.0.1
$ npm install -g utf-8-validate@^5.0.2
$ npm install -g elm-doc-preview
npm WARN ws@7.2.3 requires a peer of bufferutil@^4.0.1 but none is installed. You must install peer dependencies yourself.
npm WARN ws@7.2.3 requires a peer of utf-8-validate@^5.0.2 but none is installed. You must install peer dependencies yourself.

elm-doc-preview seems to at least start, so I'm not sure if this is a problem or not.

rlefevre commented 4 years ago

Thanks. Could you tell me your version of Node.js and operating system please?

malaire commented 4 years ago

OS: Debian 10 "Buster" npm: 5.8.0+ds6-4 (current version from Debian repository) nodejs: 10.15.2~dfsg-2 (current version from Debian repository)

Here is also complete output from install, but only the two lines I mentioned earlier seem meaningful. First 5 lines are expected warning in Debian 10.

$ npm install -g elm-doc-preview
npm WARN npm npm does not support Node.js v10.15.2
npm WARN npm You should probably upgrade to a newer version of node as we
npm WARN npm can't make any promises that npm will work with this version.
npm WARN npm Supported releases of Node.js are the latest release of 4, 6, 7, 8, 9.
npm WARN npm You can find the latest version at https://nodejs.org/
/home/malaire/.npm-global/bin/elm-doc-preview -> /home/malaire/.npm-global/lib/node_modules/elm-doc-preview/cli.js
/home/malaire/.npm-global/bin/edp -> /home/malaire/.npm-global/lib/node_modules/elm-doc-preview/cli.js
npm WARN ws@7.2.3 requires a peer of bufferutil@^4.0.1 but none is installed. You must install peer dependencies yourself.
npm WARN ws@7.2.3 requires a peer of utf-8-validate@^5.0.2 but none is installed. You must install peer dependencies yourself.
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@2.1.2 (node_modules/elm-doc-preview/node_modules/fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@2.1.2: wanted {"os":"darwin","arch":"any"} (current: {"os":"linux","arch":"x64"})

+ elm-doc-preview@5.0.1
rlefevre commented 4 years ago

For some reason, the warnings seem to only be displayed with Node.js 10.x (not 8.x or 12.x).

Those peer dependencies are optional, so impact should be limited: https://github.com/websockets/ws/blob/6b4e2a89ffa4f0b866ba8078868b45d2c4eea6d7/package.json#L37

Anyway I found another dependency issue with the recent 5.x version, so I'm preparing a patch version that also fixes this one. Could you unzip then test to install the following pre version please?

elm-doc-preview-5.0.2-rc1.zip

$ cd /tmp
$ mkdir test-doc-preview
$ wget https://github.com/dmy/elm-doc-preview/files/4493641/elm-doc-preview-5.0.2-rc1.zip
$ unzip elm-doc-preview-5.0.2-rc1.zip
$ npm install elm-doc-preview-5.0.2-rc1.tgz
$ npx elm-doc-preview

Thank you!

malaire commented 4 years ago

With pre-version I now get these messages instead of "requires a peer" warnings, so seems to be working?

> bufferutil@4.0.1 install /tmp/test-doc-preview/node_modules/bufferutil
> node-gyp-build

> utf-8-validate@5.0.2 install /tmp/test-doc-preview/node_modules/utf-8-validate
> node-gyp-build
rlefevre commented 4 years ago

Ok. So I looked at little more about those. They are performance optimizations for websockets: https://github.com/websockets/ws#opt-in-for-performance-and-spec-compliance

elm-doc-preview has a quite light usage of websockets, and mainly server -> client so I expect these optimizations to have no or negligible impact on elm-doc-preview performance.

As far as I can tell, there is no way to remove the warnings with Node.js 10.x about these optional peer dependencies, but they have been removed in Node.js 12.x. Also these dependencies require some C++ libraries, so I think it's better to not include them by default.

I will add a point in the README though.

tldr: the warnings will stay with Node.js 10x, you don't have to install the dependencies though.

rlefevre commented 4 years ago

I have added a note in the README: https://github.com/dmy/elm-doc-preview#installation

Thank you for the report.

malaire commented 4 years ago

ok, I guess this could be considered a bug in Node.js 10.x as message says "You must install peer dependencies yourself" when "must" is clearly wrong word to use for optional dependencies.

rlefevre commented 4 years ago

Kind of. It's more that Node.js 10.x does not recognize the syntax to declare them optional. The feature has been introduced in Node.js 12.x.