Feature Request: Limit size of the package for browser usage

[KnisterPeter]

I would like to use this package as part of a client side library. This should (not tested it) right now, but its in some ways suboptimal.
The points I would address in this feature request:

• Make the usage of dependencies optional (only require that libs during use of that cases) so that web bundlers could skip bundling them
  • bottleneck is around 59k of code size
  • get-ssl-certificate is around 83k of code size
  • node-mbed-dtls-client is a native dependency
  • dgram is node specific and should be skipped for a browser bundle
• Think on how to support tree-shaking to limit library size to only used parts

I could help to implement these and discuss solutions if this is accepted as target.

[peter-murray]

dgram functionality is not possible from client as noted and is only part of the older upnp lookups, if you don’t use discovery, it does not feature.

Node-mbed-dtls-client was only in play when I was evaluating supporting the entertainment api, it is not part of the dependencies, so not sure where this is coming from... is it in a old lock file?

Get-ssl-certificate is required to retrieving the cert from the bridge and validating the client on a local network, I guess you are looking at using the remote api functionality, so these are valid external ssl certificates?

Bottleneck was the smallest bandwidth limiting library I could find, but if you are using the remote api, thing will change a bit with respect to usage, although the overall limits are still valid to stay within the bridge limits for performance.

I prefer to not use a dependency when I can implement the same in a smaller way with low or no dependencies, but the last two give a reasonable amount of functionality that I really don’t want to have to build/maintain myself.

Can you clarify that your use case is a browser based client, using the remote api functionality? If so then I understand the desire to exclude dependencies that are not useful.

I have kept in mind that minification will be useful for browser users, so avoid doing dynamic requires etc... to ensure things will able to be minimised correctly.

I need to think on this a little, based off your answers, but we should be able to achieve some improvements for you.

[peter-murray]

I have taken a look at the get-ssl-certificate and it is really only a single file that provides the ability to read the certificate, it does use https, but it should not be coming in at a size of 83k if it is being minified (as all the other files in the package would be discarded).

How are you minifiying the code? As others in the distant past have minified this code base for the browser usage and we worked through those issues successfully in the past (one of the reasons that pushed me over to using axios over request at the time).

[KnisterPeter]

Currently I use webpack which bundles all requiredd sources. I could inspect this in more depth and comment my findings.
What I had in mind is either tree shaking (that would require using es imports instead of requires) which I guess you don't favor, or split the API into multiple entry points instead of just the main package import.
That way code splitting will have a chance to (for example) get around the ssl validation and the node https module at all.

[KnisterPeter]

Hi @peter-murray,

thanks for you comment. I'll try to answer as good as possible.

dgram functionality is not possible from client as noted and is only part of the older upnp lookups, if you don’t use discovery, it does not feature.

Since discovery is part of the exported api, dgram is as well and need a workaround for webpack.

Node-mbed-dtls-client was only in play when I was evaluating supporting the entertainment api, it is not part of the dependencies, so not sure where this is coming from... is it in a old lock file?

Good point. We can skip this. The main entry into this - the EntertainmentApi is commented out.

Get-ssl-certificate is required to retrieving the cert from the bridge and validating the client on a local network, I guess you are looking at using the remote api functionality, so these are valid external ssl certificates?

This is a server-side only think I guess. In the browser the browser itself is checking the ssl certificate.

Bottleneck was the smallest bandwidth limiting library I could find, but if you are using the remote api, thing will change a bit with respect to usage, although the overall limits are still valid to stay within the bridge limits for performance.

Thats a nice one, but I guess (at least for my usecase) this will be sufficient on server side only.
Because I'm creating a PWA which requires a valid ssl domain, I need kind of a proxy to my hue-bridge. Therefore I use the remote api and have no way around it. And to be secure with the app-credentials I deployed a server side proxy between the remove-api and my PWA. There all this makes sense, but not directly in the client I guess.

I prefer to not use a dependency when I can implement the same in a smaller way with low or no dependencies, but the last two give a reasonable amount of functionality that I really don’t want to have to build/maintain myself.

Sure, thats what I prefer also normally.

Can you clarify that your use case is a browser based client, using the remote api functionality? If so then I understand the desire to exclude dependencies that are not useful.

Does the above is enough explaination? I would like to have a PWA which requires a valid ssl domain. Therefore I cannot directly speak to the bridge, and the remote api directly does not support CORS and cannot secure the app credentials. Therefore I have a server-side part which will talk to the remote api, the client will use the server side 'mirror' (or proxy) of the api.

I have kept in mind that minification will be useful for browser users, so avoid doing dynamic requires etc... to ensure things will able to be minimised correctly.

That sounds reasonable and good. Its hard to require from variables when you want to bundle something.
But to split code and only load parts in the client when required there is the need to import/require in a promise like way.
It's not that easy to do that with plain node and if you are willing to do so, we need to spike a few ideas (inspired by webpacks require.ensure).

[peter-murray]

Thanks, that provides me with some good context.

This has been used in the browser before, I had a user utilizing it there, but that was under the 2.x versions.

I think the change to have discovery as a part of the v3 API is the change that might be now pulling in the dgram stuff from webpack. I need to take a look at this, but that would make a breaking change if it was to come out, so we would be looking at a major version number bump in that case.

If we look at the browser only uses case then you cannot access the Remote API directly, as CORS will prevent that from happening, hence your proxy service. In this case though you are not going to be instantiating a bridge, so the ssl certificate stuff would be dropped when you package your code, as it will not be used.

The project is built to support Node.js as the primary use case and others as a nice to have, so things are driven from the Node.js language support first. Unfortunately Node.js is really dragging out the migration to ES6 modules and I would be extremely cautious of creating a dual CommonJS/ES Module Package as there can be some very difficult issues you can introduce to downstream users.

I am also not a massive fan of compiling/transpiling JavaScript, but the community and browser based projects generally are. The whole point of JavaScript from me was avoiding the compilation (and verbosity) as I came from a Java background.

It may be possible to provide a stripped down API separate to v3 that is more browser friendly, but I don't want a nightmare of maintaining different sets of documentation for such a thing (along with split test suites).

I will take a bit of time to look at what a stripped down browser client might look like and set up some tests with webpack and see what starts to fall out. I am happy to continue to work on this to find something that works and makes sense.

[peter-murray]

So some quick findings:

With a modified v3 api entry point excluding the discovery features, webpack will generate a 521kb module.

I applied a patch to remove bottleneck and that got it down to 462kb, not the level of change I was expecting, considering that is the largest dependency (outside of axios). Where did you get your numbers from that you reported in the opening of this? It is still ballpark range, but not matching up with what I expected.

I need to look at what is cuasing the library to be so large with packed, as I would have thought it would have come in a little lighter and I need to plan to spend time in the right areas where we can get the best return.

[peter-murray]

In your use case, you have a proxy server. In the browser client side, you are not going to be creating a bridge instance, as you are talking to your proxy, not a bridge.

Are you emulating the actual API endpoints of a Hue bridge in your proxy and instantiating a bridge instance in the client browser code? If not how are you doing this interaction, do you have your own custom endpoints and just passing the JSON payloads from the library?

The addition of the ability to serialize to JSON and then reconstitute back to a model based object that was added in 4.x for your use case would only require the model based part of the code base in the client, not all the API components which of course are tied in to the more low level Node.js stuff.

I think a lot of the size (just reading through the webpacked file) appears to be coming from various security and crypto components, and axios will be dragging in a lot of this too.

[KnisterPeter]

Are you emulating the actual API endpoints of a Hue bridge in your proxy and instantiating a bridge instance in the client browser code? If not how are you doing this interaction, do you have your own custom endpoints and just passing the JSON payloads from the library?

The addition of the ability to serialize to JSON and then reconstitute back to a model based object that was added in 4.x for your use case would only require the model based part of the code base in the client, not all the API components which of course are tied in to the more low level Node.js stuff.

I do exactly this. My server side does use the whole remove api while the client side will require only the deserialized model. The server serializes the json.

I think a lot of the size (just reading through the webpacked file) appears to be coming from various security and crypto components, and axios will be dragging in a lot of this too.

It would be great to have a 'lite' api, or just a model api. This should trim down the package size a lot.

[KnisterPeter]

This is the output of the webpack-bundle-analyzer of a production build of my app.

[KnisterPeter]

Here are additional outputs of the whole node_modules folder:

When adding node-hue-api to the client:

Without node-hue-api:

[peter-murray]

So I am on the fence with the model either becoming another module altogether (i.e. node-hue-api-model) or just being able to require it as a top level object from the index.js of the node-hue-api module as it is.

If extracted to completely standalone it complicates the build/dependencies and development to a degree, if included, you will still have the dependencies showing up, but when webpacked, they should all be discarded.

I just ran a quick test of webpack on the model itself and it comes in at 83kb if targeted directly for packing into a minimized module.

What is your personal preference to this? Would having something like ;

const model = require('node-hue-api').model.v3;

work if I pulled that up from the existing v3 object in index.js or do you need something cleaner with respect to breaking it up.

Is having this webpacked in a dist directory and referenced as a module from the library (i.e. it would be native node.js module by default Common JS main, but you could import the model using new ES module references) required, or would you be happy to pack it on your side?

[KnisterPeter]

Breaking up the model sound very good. I would say for ease of api handling keep it in the same package but the require statement need to be a bit different. This should work for webpack:

const model = require('node-hue-api/model').v3;

If you have it directly on the main, webpack will still bundle all the api, because commonjs is by its nature of dynamic not tree shakeable.
To not break the current api and have it still in the v3 version, you could reexport it still from the main module. That way there is only a new 'more client' friendly version to require the model.

I'm also more than happy to bundle/pack it on my side, webpack will do nonetheless. No need to prebundle it.

[peter-murray]

I cannot find something I am happy with here that works across the mix of things we have in play here:

• TypeScript definitions
• Node.js CommonJS modules
• ESM for the browser, that really only targets the Model

I have toyed with pulling out the model, types and placeholders in to a separate node module, but event then I am faced with having to invert all the code and rewrite it to use ESM and then compile back down to CJS. Due to the number of files involved here, that is a fairly heavy amount of work and something that does not really feeel worth the effort until Node.js properly catches up with ESM modules and I can cut support for the older LTS versions.

The reason I lean this way is that browser usage of the library is significantly smaller than Node.js usage.

Trying to tie in the TypeScript definitions is also tricky as the modules for Node.js and Browser end up being different, and I cannot find any easy way to support the generated TypeScript definitions across different modules (unless you write everything in TypeScript, which is a massive undertaking in itself, and not something I would enjoy doing).

That leaves me with a possibility of generating bundled files in a dist directory that would be contained in the tarball that published to npm. I can generate the model module at dist/model.js that you could consume, but not sure how easy that would be in your development process. I have seen some examples of older node libraries doing this type of thing, but I am not sure it standard practice these days.

Attached is an example of webpacked code that would be in the dist directory of a published npm package:
dist.zip

Would that work for you?

I really need to wait until we get an LTS version of Node.js that supports ESM before I want to invest a large amount of time in converting to ESM in the code base, or there is a larger pool of users requiring browser compatible code (which would require the library to be split up across the parts that are compatible with the browser and the rest).

If on the other hand you want to do an example branch of the code with the changes then I am happy to review that and kick the tyres in how it would work in practice.

[KnisterPeter]

Hi @peter-murray,
I also guessed thats it not that easy to get all the thing to work in a clean and reasonable way.

I know you prefer node.js as primary use case for this lib and thats totally fine (my case is in some way special).
Also the typescript (jsdoc) thing is quite hard and will get harder if these are multiple modules.

I also could understand that converting to typescript is not what you want and in the end you should have joy building and maintaining this lib. Otherwise we all would suffer 😉

I'm totaly fine with a model.js in a dist folder. Thats right now the right way to do it I think.
Whenever node (problably 12.x or 13) will have decent support for ESM we could reevaluate this - with ESM this would be quite easy because of proper tree shaking.

If you could provide a build with a separate module.js I'll give it a shot and see how far I can go with this. That would be a big improvment for me.

Thanks for all the effort you invested here.

[peter-murray]

There is a version of 4.0.2-alpha.1 or @next available with a dist/model.js webpacked file to try.

[KnisterPeter]

Great! I'll give it a try.
By the way: Are you willing to push that to a branch so I could fork and play around with that? I would propose to use rollup for bundling libraries (webpack is more for applications).

In addition I would love to see the model.js in the package root, so the import would be more like node-hue-api/model.js instead of node-hue-api/dist/model.js. That could be created with package.json files and a bit of copy at prepublish step.

[peter-murray]

The problem is that the index.js is kind of locked in CJS and will be in CJS format until Node.js supports ESM natively, which is part of the problem you have with tree shaking.

Lifting the model up is not too much of an issue, but, it would be in a CJS module, which exposes you to the same problem you are facing.

I tried rollup, but unless you are writing in ESM module syntax, the results do not look like what you are after. This also results is a mess with TypeScript definitions not matching up unless you identically have the rollup modules exposed under modules in the package.json generating off the same code, which leads us back to the problems of you not being able to treeshake the Node.js parts you do not want.

[peter-murray]

The existing webpack stuff is on the master branch currently, just not the webpack and webpack cli as dependencies. there is a script entry for generating the model.js file once tlhose two dev dependencies are installed (or globally available)

[peter-murray]

First commit in 5.0.0 alpha release to move the discovery out of the v3 API to remove some references that are not needed in the web app use case, f085599

It will take until release 6.x before I can complete the removal of all of this, but it is deprecated now.

[KnisterPeter]

Thank you. Thats great news 😃

[peter-murray]

@KnisterPeter 👋 , I have just completed the extraction of the model to a pure TypeScript library that is used to provide CommonJS and ESM module systems (https://github.com/peter-murray/hue-bridge-model).

I am preparing some changes to the way that this library will build, but other than the extraction of the library for the model, was there anything else here that was problematic to you?