Closed danopia closed 3 years ago
Hey @danopia 👋
A solution with Deno.inspect
sounds fantastic, thanks so much for doing all the legwork.
I think the quote changes are acceptable - not precious about when / what yields major version upgrades, so we can just chuck it into a 4.0.0
to be safe in case anyone is relying on specific quotes in their tests.
Feel free to raise the PR and we can get that reviewed and merged :tada:
Looking a lot better 🤩 https://deno-visualizer.danopia.net/dependencies-of/https/deno.land/x/superdeno@4.0.0/mod.ts
Issue
I noticed that an NPM library was being used to provide node's
util
functionality, which brings in a total of 16 NPM packages. (22 vs 6 going by the badges that were just added 😄 )It seems like this import is easily avoided by leveraging Deno's std library.
Details
After searching the codebase I determined that the
util
import from NPM was only being used to polyfillutil.inspect()
for error messages. I've prepared a patchset that removes theutil
NPM package in favor ofDeno.inspect
: https://github.com/asos-craigmorten/superdeno/compare/main...danopia:use-deno-inspectGoing by the test suite, the only difference I observed was that Deno.inspect uses different quoting rules than the util.inspect from NPM. So I had to adjust some quotes in the test suite. I suppose it's possible that consumers are testing for exact messages too but overall the change doesn't seem very disruptive to me.
I also have a mutually exclusive patchset that uses Deno's
/std/node
polyfile forutil.inspect
but it doesn't seem to give anything over usingDeno.inspect
directly. Here it is anyway: https://github.com/asos-craigmorten/superdeno/compare/main...danopia:use-std-utilI'm prepared to open a PR with either patchset if this looks agreeable.
Here's the before / after of the package dependency tree: