paho.mqtt.javascript
paho.mqtt.javascript copied to clipboard
modern JavaScript workflow
I spoke with @icraggs today about this project, and I'd like to help move it forward. We need to get MQTT v5 support going, but before that, I'd prefer to udpate the workflow: I'd like to see this fully integrated with npm and Node.js, using a typical toolchain to bundle for the browser. It's not a great developer experience for someone to have to install a Java toolchain to build a JS project.
Here's what needs to happen:
-
package.json
- [ ] Add missing development dependencies (
devDependencies
); ensure no production dependencies are contained therein (and vice-versa) - [ ] Upgrade
dependencies
- [ ] Replace any dependencies that are not Node.js-compatible
- [ ] Add missing fields:
files
,author
,keywords
,main
,homepage
,scripts
(see below),version
(see below), etc. - [ ] Modify
description
to reflect that it is, indeed, the actual Paho JS client - [ ] Leverage the
browser
field to point to bundle - [ ] Add
repository
field (Is it here orgit.eclipse.org
? Please advise) - [ ] Add
license
field. I expect that's EPL-1.0 as per the SPDX list
- [ ] Add missing development dependencies (
- Linting
- [ ] Lint JS and docstrings using the popular and flexible ESLint. Do not need Closure compiler. Configuration and plugins TBD, but semistandard is a reasonable place to start.
- [ ] (Optional) Lint Markdown using Remark or similar tool
- [ ] Add linting scripts to
scripts
prop ofpackage.json
(should also be invoked viatest
script)
- Tests & CI
- [ ] Execute tests via
test
prop ofscripts
- [ ] Update
.travis.yml
to build against supported versions of Node.js (I'm thinking v6.x or newer; v4.x will drop out of LTS soon) - [ ] Run tests in headless Chrome
- [ ] Choose a target browser (e.g. IE11) in preparation for eventually running tests against it (on SauceLabs, for instance)
- [ ] Execute tests via
- Build & bundling
- [ ] Remove Maven and associated
.xml
files - [ ] Bundle for web with (probably?) Webpack
- [ ] Prepend copyright banner
- [ ] Minify via Uglify plugin
- [ ] Add bundler to
build
script
- [ ] Remove Maven and associated
- Distribution
- [ ] Add
build
script toprepublishOnly
script; ensure distfiles are listed infiles
- [ ] Ensure bundle / dist files stay out of version control
- [ ] Do not publish
.zip
archives within the tarball on npm :smile: - [ ] Use SemVer if not already
- [ ] Create
.zip
file? Not sure how these are used..zip
archives are available on GitHub, and tarballs of published packages on npm. Please advise. - [ ] Add dist file to
browser
field as mentioned earlier
- [ ] Add
- Documentation
- [ ] Run JSDoc via e.g.,
doc
npm script (we can publish this automatically on GitHub Pages at https://eclipse.github.io/paho.mqtt.javascript unless the API docs need to live where they currently live) - [ ] Add
doc
script to a completebuild
script - [ ] (Optional) Add
README.md
to the API docs - [ ] Update
README.md
; add any material changes to install process - [ ] Update
CONTRIBUTING.md
likewise (maybe move to.github/
along withCODE_OF_CONDUCT.md
); it's already out-of-date - [ ] Figure out what to do with
about.html
- [ ] Note browser and Node.js compatibliity
- [ ] Run JSDoc via e.g.,
Given I have entirely too much experience doing this stuff, it looks like a lot more work than it is. The two "hard" parts are likely going to be:
- Running tests in headless Chrome, because I haven't set that up before. Hoping to avoid having to pull in Karma to do it
- Adding Node.js support & bundling for browser could be painful
IMO it's best to knock this out all at once. I am completely new to this codebase, so there may be something I've missed. None of this should materially affect production usage of this package apart from potentially a different way to install it.
This looks like a good plan @boneskull (although I am new to this library, I am not completely aware of its state). Would you be open to PRs?
It appears jasmine needed an upgrade. Unfortunately, it's not backwards-compatible, so upgrading the tests from v1.x to v3.x syntax needs to happen. The jasmine-node module, which this project consumes, hasn't seen a new version in four (4) years.
...also, the tests are written in such a way that a newer version is especially painful to use without pulling in async or promisifying stuff. otherwise it'll devolve into "callback hell", because some tests make multiple asynchronous assertions, which must happen in a certain order.
Hi @boneskull This all sounds great, the Javascript client is definitely in need of some love and your plan looks pretty solid.
As you've already noticed the Jasmine tests are in a bit of a stuck state due to the out of date libraries. I was never able to find a nice way of converting them to using the newer version, do you think it would be better for us to bite the bullet and to re-write them from scratch using async?
Let me know if you need anything from me to accomplish this, and I'll process any PRs as soon as they come in.
As you've already noticed the Jasmine tests are in a bit of a stuck state due to the out of date libraries. I was never able to find a nice way of converting them to using the newer version, do you think it would be better for us to bite the bullet and to re-write them from scratch using async?
IIRC async expects the first parameter to any given callback be an error, if present, so maybe that wouldn't work anyway, because the client's callbacks don't all conform to this style (see #141). So, that'd require breaking API changes--likely not the first thing on the TODO list.
Unless someone has written a codemod for this already, there's no recourse but to rewrite large swaths of the tests, I'm afraid. It's not easy, but we can:
- Try to eliminate cases where we are making multiple assertions per test, and split them up
- DRY; there is duplication we can zap
- Consider the value of each test, and eliminate tests which offer little, or rewrite others into unit tests
I expect a significant reduction in LoC w/o loss of coverage, which is a plus. Probably good to start with determining that coverage % as a baseline, and ensuring we don't drop below it.
@kbariotis yes, we are always open to PRs. Also, I've been told that @boneskull may not be able to contribute to the project as he wished, so any contributions would be even more valuable.
@icraggs @jpwsutton I am up for helping out with this, I have experience setting up Javascript Testing and Build tasks
Is there any progress on the bundling for the web bullet?
I started to structure the main file (class + import), used webpack, changed vars to let/conf, cleaned the eslint errors and adjusted some small things in the utility to being able to run it as chrome app (to possibly try tcp there soon). The libraries should be the same as in eclipse unide for piggyback CQs. Are you interested in a PR? https://github.com/eclipse/paho.mqtt.javascript/pull/158