noVNC icon indicating copy to clipboard operation
noVNC copied to clipboard

websock.js out of sync with upstream

Open CendioOssman opened this issue 8 years ago • 9 comments

We changed websock.js to use ES6 module syntax recently, which got us out of sync with upstream. We should merge things back so we don't start drifting apart again.

CendioOssman avatar Apr 19 '17 11:04 CendioOssman

Is this something we want to do before a 1.0 release you think?

samhed avatar Oct 24 '17 20:10 samhed

No, this is internal and doesn't affect any interface.

CendioOssman avatar Oct 25 '17 08:10 CendioOssman

Hei,

Where is upstream? And how is this affected by #1013 ?

juanjoDiaz avatar Feb 04 '18 02:02 juanjoDiaz

Hei,

Where is upstream?

Upstream is here: https://github.com/novnc/websockify/blob/master/include/websock.js

And how is this affected by #1013 ?

We'll have to sort out those changes as well when syncing things up. It's already sufficiently out-of-sync that you don't need to worry about it for your work though.

CendioOssman avatar Feb 05 '18 12:02 CendioOssman

Have you considered publishing websock to NPM and adding it to noVNC as a dependency? That would facilitate a lot keeping it in sync with upstream.

juanjoDiaz avatar Jul 19 '19 10:07 juanjoDiaz

Publishing it to NPM is definitely interesting. But adding it as a dependency is not. We like to make sure that noVNC is useful right away after a checkout. That's why we have the stuff in the vendor/ directory rather than pulling it from NPM.

CendioOssman avatar Jul 22 '19 13:07 CendioOssman

Can you elaborate a bit on what's the problem of pulling dependencies from NPM? I mean, what's the downside of having to run npm install after downloading the project. Node.js is already used for many things in the project.

Using Node.js as the main development tool would allow:

  • Replacing some of the utils scripts in Perl, Python or Bash by Node
  • Replacing the launch.sh by a cross-OS script (websockify could also be published as an NPM package and pulled as a dependency instead of using git).
  • New scripts could be added to package.json to serve the UI, launch websockify, and whatever else is needed more conveniently

I think that it could be a more unified developer experience.

juanjoDiaz avatar Jul 22 '19 14:07 juanjoDiaz

The concern is not developers, but users. E.g. even our own usage on novnc.com would stop working at that point as it relies on being able to just check out the code and nothing more.

CendioOssman avatar Jul 22 '19 14:07 CendioOssman

I think that users still have to set up a web server in front of the app. So it's probably the same to set up the server on the whole project (possibly exposing all the utils, configs and such). Or having to run npm build and then exposing a dist folder.

Of course, it's just a matter of opinion. If you want to keep things as they are, then you just can't use package managers and need to keep syncing things manually 🙂

juanjoDiaz avatar Jul 23 '19 06:07 juanjoDiaz