Closed bertho-zero closed 6 years ago
The current size appears to be 188K, of which:
extsprintf
dependencycore-util-is
dependencyassert-plus
dependencyIs 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:
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:
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:
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?
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
andhasCauseWithType
methods have been addedSError
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: https://github.com/joyent/node-verror/issues/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
@bertho-zero thank you, this smaller version of the package is very useful
I find it unfortunate that an error management package is so imposing, is there no way to reduce its size?