cmorten/superdeno

[CHORE] Use Deno's inspect instead of one from NPM

danopia opened this issue · 2 comments

danopia commented

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 polyfill util.inspect() for error messages. I've prepared a patchset that removes the util NPM package in favor of Deno.inspect: main...danopia:use-deno-inspect

Going 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 for util.inspect but it doesn't seem to give anything over using Deno.inspect directly. Here it is anyway: main...danopia:use-std-util

I'm prepared to open a PR with either patchset if this looks agreeable.

Here's the before / after of the package dependency tree:

Screenshot 2021-02-10 at 20 21 54
Screenshot 2021-02-10 at 21 00 57

cmorten commented

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 🎉