Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reduce dependencies #116

Closed
cztomsik opened this issue Mar 19, 2020 · 8 comments · Fixed by #182
Closed

reduce dependencies #116

cztomsik opened this issue Mar 19, 2020 · 8 comments · Fixed by #182

Comments

@cztomsik
Copy link

cztomsik commented Mar 19, 2020

Hey, I love this effort but maybe we could replace npmlog with something lighter?

    ├─┬ npmlog@4.1.2
    │ ├─┬ are-we-there-yet@1.1.5
    │ │ ├── delegates@1.0.0
    │ │ └─┬ readable-stream@2.3.7
    │ │   ├── core-util-is@1.0.2
    │ │   ├── inherits@2.0.4 deduped
    │ │   ├── isarray@1.0.0
    │ │   ├── process-nextick-args@2.0.1
    │ │   ├── safe-buffer@5.1.2 deduped
    │ │   ├─┬ string_decoder@1.1.1
    │ │   │ └── safe-buffer@5.1.2 deduped
    │ │   └── util-deprecate@1.0.2
    │ ├── console-control-strings@1.1.0
    │ ├─┬ gauge@2.7.4
    │ │ ├── aproba@1.2.0
    │ │ ├── console-control-strings@1.1.0 deduped
    │ │ ├── has-unicode@2.0.1
    │ │ ├── object-assign@4.1.1
    │ │ ├── signal-exit@3.0.2
    │ │ ├─┬ string-width@1.0.2
    │ │ │ ├── code-point-at@1.1.0
    │ │ │ ├─┬ is-fullwidth-code-point@1.0.0
    │ │ │ │ └── number-is-nan@1.0.1
    │ │ │ └── strip-ansi@3.0.1 deduped
    │ │ ├─┬ strip-ansi@3.0.1
    │ │ │ └── ansi-regex@2.1.1
    │ │ └─┬ wide-align@1.1.3
    │ │   └── string-width@1.0.2 deduped
    │ └── set-blocking@2.0.0

And maybe we could improve it even further, I've just installed keytar and this is my npm ls (I only have one other dependency which is dep-free)

└─┬ keytar@5.4.0
  ├── nan@2.14.0
  └─┬ prebuild-install@5.3.3
    ├── detect-libc@1.0.3
    ├── expand-template@2.0.3
    ├── github-from-package@0.0.0
    ├── minimist@1.2.5
    ├─┬ mkdirp@0.5.3
    │ └── minimist@1.2.5 deduped
    ├── napi-build-utils@1.0.2
    ├─┬ node-abi@2.15.0
    │ └── semver@5.7.1
    ├── noop-logger@0.1.1
    ├─┬ npmlog@4.1.2
    │ ├─┬ are-we-there-yet@1.1.5
    │ │ ├── delegates@1.0.0
    │ │ └─┬ readable-stream@2.3.7
    │ │   ├── core-util-is@1.0.2
    │ │   ├── inherits@2.0.4 deduped
    │ │   ├── isarray@1.0.0
    │ │   ├── process-nextick-args@2.0.1
    │ │   ├── safe-buffer@5.1.2 deduped
    │ │   ├─┬ string_decoder@1.1.1
    │ │   │ └── safe-buffer@5.1.2 deduped
    │ │   └── util-deprecate@1.0.2
    │ ├── console-control-strings@1.1.0
    │ ├─┬ gauge@2.7.4
    │ │ ├── aproba@1.2.0
    │ │ ├── console-control-strings@1.1.0 deduped
    │ │ ├── has-unicode@2.0.1
    │ │ ├── object-assign@4.1.1
    │ │ ├── signal-exit@3.0.2
    │ │ ├─┬ string-width@1.0.2
    │ │ │ ├── code-point-at@1.1.0
    │ │ │ ├─┬ is-fullwidth-code-point@1.0.0
    │ │ │ │ └── number-is-nan@1.0.1
    │ │ │ └── strip-ansi@3.0.1 deduped
    │ │ ├─┬ strip-ansi@3.0.1
    │ │ │ └── ansi-regex@2.1.1
    │ │ └─┬ wide-align@1.1.3
    │ │   └── string-width@1.0.2 deduped
    │ └── set-blocking@2.0.0
    ├─┬ pump@3.0.0
    │ ├─┬ end-of-stream@1.4.4
    │ │ └── once@1.4.0 deduped
    │ └─┬ once@1.4.0
    │   └── wrappy@1.0.2
    ├─┬ rc@1.2.8
    │ ├── deep-extend@0.6.0
    │ ├── ini@1.3.5
    │ ├── minimist@1.2.5 deduped
    │ └── strip-json-comments@2.0.1
    ├─┬ simple-get@3.1.0
    │ ├─┬ decompress-response@4.2.1
    │ │ └── mimic-response@2.1.0
    │ ├── once@1.4.0 deduped
    │ └── simple-concat@1.0.0
    ├─┬ tar-fs@2.0.0
    │ ├── chownr@1.1.4
    │ ├── mkdirp@0.5.3 deduped
    │ ├── pump@3.0.0 deduped
    │ └─┬ tar-stream@2.1.2
    │   ├─┬ bl@4.0.2
    │   │ ├─┬ buffer@5.5.0
    │   │ │ ├── base64-js@1.3.1
    │   │ │ └── ieee754@1.1.13
    │   │ ├── inherits@2.0.4 deduped
    │   │ └─┬ readable-stream@3.6.0
    │   │   ├── inherits@2.0.4 deduped
    │   │   ├── string_decoder@1.1.1 deduped
    │   │   └── util-deprecate@1.0.2 deduped
    │   ├── end-of-stream@1.4.4 deduped
    │   ├── fs-constants@1.0.0
    │   ├── inherits@2.0.4
    │   └─┬ readable-stream@3.6.0
    │     ├── inherits@2.0.4 deduped
    │     ├── string_decoder@1.1.1 deduped
    │     └── util-deprecate@1.0.2 deduped
    ├─┬ tunnel-agent@0.6.0
    │ └── safe-buffer@5.1.2
    └── which-pm-runs@1.0.0

BTW: I'd love to use prebuild-install myself (for various OSS projects) but there's no way I'd just splat there extra 80 deps. I'm currently using node-fetch and so far it's enough for my case (it's just one dep and it can follow redirects and save to file stream) - just saying as something to think about.

BTW2: I'm willing to help if you want to.

@vweevers
Copy link
Member

We use npmlog so that our logs cleanly interweave with npm's own logs, and to respect npm's log level.

If there's an alternative, lighter package that does the same, I wouldn't mind swapping npmlog. Although personally I have no problem with the number of dependencies (note btw that 20 of the 80 above are deduped).

@cztomsik
Copy link
Author

Good point with deduping but there's still a lot of other deps.

And I see your reasoning but it feels hard to swallow. Testing process.env.npm_config_loglevel is super easy and you don't seem to use any of the advanced npmlog functionality - and I'd generally advise against as I'm not interested in colors, progress bars and whatever, I just want this to download prebuilt binary and that's it.

This has potential to be one of the most depended pkgs and it's important to keep deps low because otherwise the current situation is only going to get worse (npm i jest is 1k pkgs now)

So, I don't have any drop-in replacement but I personally think you don't need it.

@vweevers
Copy link
Member

I'm not interested in a discussion about the merits of npm packages atm.

You are right that it's not actually doing that much. A PR is welcome to replace it, provided that you don't strip it down too much because re: "I just want this to download prebuilt binary and that's it", logs have proven themselves to be useful many times.

@cztomsik
Copy link
Author

Sorry if that was offensive, it was not intentional. The whole point of this issue is that this project is advised as minimal and so I was very surprised by the actual number of deps.

@vweevers
Copy link
Member

That's fair. We called it "minimal" because prebuild-install was split off from the larger prebuild cli.

This has potential to be one of the most depended pkgs

I'm actually hoping it eventually dies out because there is a better alternative (zero deps).

@cztomsik
Copy link
Author

cztomsik commented Mar 21, 2020

Oh wow, that looks great! I will definitely have a better look.

But I'm not sure I can use it for my project - my extension is in rust and I'm building it myself, using custom node.js script (I need some codegen too) - and then the result is put into /target/ and I need this to be built on different platforms - so I've started experimenting with github actions which looks promising and I'm able to build all the binaries and the original idea was to put it into github releases but this indeed look better - I'm just not sure how to combine it. At first look it seems to be too much coupled to the C++ and gyp

(but the idea is very interesting!)

@vweevers
Copy link
Member

At first look it seems to be too much coupled to the C++ and gyp

Yeah, it is. Keeps the scope small and the code clean. You might be interested in this idea: prebuild/prebuildify#7

@ralphtheninja
Copy link
Member

@cztomsik You might want to take a peek at https://github.com/deltachat/deltachat-node , it's a node module for native code, but based on a lib that comes from the rust core module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants