TritonDataCenter/node-verror

Bundle size

bertho-zero opened this issue ยท 25 comments

I find it unfortunate that an error management package is so imposing, is there no way to reduce its size?

image

The current size appears to be 188K, of which:

  • 68K is the extsprintf dependency
  • 36K is the core-util-is dependency
  • 28K is the assert-plus dependency
  • 24K is the README
  • 12K is the source

Is the download time over 2G edge and emerging 3G relevant?

This library isn't really built specifically for constrained environments like web applications being delivered over lossy wide area networks. Rather, it's aimed at server-side use in a Node environment where a few extra kilobytes of source code for a formatting library or an assertion framework are not an issue.

[Responding to a comment that appears to have been deleted.]

I don't think it's right to say we don't care about other environments, but rather, the people working on verror are not themselves working in those environments, so we don't focus efforts there. Further, we're not willing to make the module worse for the server environments for which it was created in order to apply the module more broadly. But we would certainly consider specific proposals! Do you have a specific proposal for improving the size here? Can you elaborate on the specific problem, too? As I asked before, is it really 2G or 3G download times that's relevant?

It's especially that I used to use verror on the server side but it uses the util module of node, so it is not client compatible. I do not look at the code but the dependencies seem to me to weigh down the package for not much.

in my experience, it's the same for many packages. They grow and grow and grow and last time I wantesd to install a dummy server at my workplace, it grew from 10MB with deps installed to over 120MB. The only thing that damn dummy does is serve two JSON files and respond to five more REST calls. Imho, that's very wrong and a big problem with the community in general. Maybe VError can make a difference by becoming more light-weight again. I am not talking about removing features, just evaluating heavy dependencies and their gains, maybe choosing a different lib, implementing the needed functionality yourself or even creating a new, lean package specifically for that functionality.

Thanks for the input! I think the steps missing here are some research to see where space can be saved, a specific suggested change, some quantification of the improvement, and an explanation of why the tradeoffs might be worthwhile.

Thanks for turning this into something concrete.

There are a bunch of problems with the PR:

  • We use cr.joyent.us for changes, not pull requests. Please use cr.joyent.us for future revisions.
  • The PR restyles a large chunk of the software, for no apparent reason, and inconsistently with the rest of the library.
  • This library currently supports Node going back to at least 0.10. We can't use a lot of the newer ES syntax here. We also don't want to change the engines to report ">4".
  • It looks like the implementation of some dependencies has been incorporated directly into this module. That doesn't seem like the right approach. If the dependency is too large for what it does, we should fix that, not duplicate the pieces that we need.
  • The PR has changed the implementation of some of the dependencies, but not provided any explanation of the semantic changes of the dependencies for us to know that those changes are the same.

There are other issues as well here, but these are pretty significant (and, in some cases, deep). And for such significant changes, there's no answer to my original question of "why the tradeoffs might be worthwhile".

We're pretty happy with the dependencies we have today. It would be far more preferable to reduce their size, if appropriate, than to change to alternate implementations or paste their implementations into this module.

I do not understand the desire to deprive this package of lightness and additional compatibility.

The goal of this PR is to make the package compatible for all JS environments, and significantly reduce its size to not unnecessarily increase the size of the final bundles.

There is no breaking changes, I just replaced some dependencies with lighter and removed assert-plus which brings nothing more than 5 functions each containing only one ===.

There is no new ES syntax and after checking all dependencies are compatible with older versions of node.

Since there are no breaking changes and people who are in the outdated 0.10 version of node will probably never update neither node nor their dependencies we can very well release a major version without generators .

To summarize:

  • no breaking changes
  • more compatibility
  • lighter
  • no more constraints

I just forgot to update the tests to stop using any uninstalled dependencies.

I took a look at the updated PR. You have not addressed most of the issues I mentioned before:

  • As I mentioned both above and in #59, we don't use PRs in this repo.
  • The PR restyles most of the code for no apparent reason.
  • The PR reimplements a bunch of assert-plus. We have dozens or hundreds of modules that use assert-plus. It doesn't really make sense for us to reimplement it in each module (as you've done here with the new lib/assert.js).

I created an account on Gerrit, added an SSH key, tested the connection but I can not clone the project. I have an error "Project not found: node-verror".

I can keep assert-plus, it's not a big dependency.

EDIT: assert-plus uses assert, util, and stream. These are 3 Node modules, so it is necessary to replace it to have a universal package.

The git repositories on cr.joyent.us have the same names as they do on Github, including the organisation name; i.e., in this case, joyent/node-verror rather than just node-verror.

@jclulow Thank you, I was able to clone the project.

+1 for browser compatibility of this package.

@bertho-zero did you happen to publish a lighter version of verror? I'd almost be tempted to remove support for sprintf args too given template strings now exist, maybe worth publishing a verror-browser or verror-lite package?

garyo commented

I'd love to use this, but can't since my code needs to work in browser and node.js. Would be great to have a light version that works in browser!

I just published @openagenda/verror, it's a complete rewrite with some changes:

  • Rewritten with ES classes
  • Is now browser compatible, no more dependencies linking it to Node
  • findCauseByType and hasCauseWithType methods have been added
  • SError class has been removed

https://www.npmjs.com/package/@openagenda/verror

BREAKING CHANGE: The VError creation without new keyword is no longer available.

And I fixed this issue: #78

@bertho-zero I can't seem to access the repo on BitBucket -- says "We can't let you see this page"?

The reason I wanted to take a look is because I'm getting this error when trying to bundle with Webpack:
"export 'default' (imported as 'VError') was not found in '@openagenda/verror'

Also, any plans to expose TypeScript types?

@bertho-zero I noticed the esm/verror.js file pointed to by "module" in package.json contains:

module.exports = VError;
module.exports.VError = VError;
module.exports.WError = WError;
module.exports.MultiError = MultiError;

I had expected to see ESM exports rather than CJS?

@richardscarrott It's part of the monorepo of the company I work for, I can export it but I don't know TypeScript.

@bertho-zero if you were to put it on GitHub I'd be happy to send a PR to fix the ESM exports and add TS types -- I just manually changed the exports locally and found it'll save us 20.59KB (after minification and gzip) so really keen to get this in!

I just published a version 2.1.4 with ESM exports: https://bundlephobia.com/result?p=@openagenda/verror@2.1.4

image

@bertho-zero thank you, this smaller version of the package is very useful