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 #433

Open
1 of 5 tasks
icebob opened this issue Dec 9, 2018 · 48 comments
Open
1 of 5 tasks

Reduce dependencies #433

icebob opened this issue Dec 9, 2018 · 48 comments

Comments

@icebob
Copy link
Member

icebob commented Dec 9, 2018

Due to the recent event-stream vulnerability issue, I'm thinking to reduce the used NPM dependencies in the Moleculer core.
Currently it is the current dependency tree (29 modules):

`-- moleculer@0.13.4
  +-- args@5.0.0
  | +-- camelcase@5.0.0
  | +-- chalk@2.4.1 deduped
  | +-- leven@2.1.0
  | `-- mri@1.1.1
  +-- bluebird@3.5.2
  +-- chalk@2.4.1
  | +-- ansi-styles@3.2.1
  | | `-- color-convert@1.9.3
  | |   `-- color-name@1.1.3
  | +-- escape-string-regexp@1.0.5
  | `-- supports-color@5.5.0
  |   `-- has-flag@3.0.0
  +-- es6-error@4.1.1
  +-- eventemitter2@5.0.1
  +-- fastest-validator@0.6.12
  +-- glob@7.1.3
  | +-- fs.realpath@1.0.0
  | +-- inflight@1.0.6
  | | +-- once@1.4.0 deduped
  | | `-- wrappy@1.0.2
  | +-- inherits@2.0.3
  | +-- minimatch@3.0.4
  | | `-- brace-expansion@1.1.11
  | |   +-- balanced-match@1.0.0
  | |   `-- concat-map@0.0.1
  | +-- once@1.4.0
  | | `-- wrappy@1.0.2 deduped
  | `-- path-is-absolute@1.0.1
  +-- ipaddr.js@1.8.1
  `-- lodash@4.17.11

There are 3 libs which have too much dependencies:

  1. args (4 other deps)
    It is used in Moleculer Runner to parse arguments
  2. chalk (6 other deps)
    It is used to coloring some log messages
  3. glob (12(!) other deps)
    It is used in Moleculer Runner & ServiceBroker to load services from folders.
  4. vorpal
    It's a dev dependency but it is not maintained & contains some vulnerabilities.

Possible alternatives:

Important to keep the current functionality, so the changes don't cause breaking changes!

args

chalk

glob

vorpal

  • ?
@icebob
Copy link
Member Author

icebob commented Dec 9, 2018

@lukeed do you have any awesome (fast & no deps) lib to replace the huge glob lib? 😄

@lukeed
Copy link

lukeed commented Dec 9, 2018

Hey, haha, I helped work on tiny-glob. It's the one I use

@darky darky mentioned this issue Apr 25, 2019
@tinchoz49
Copy link
Contributor

https://github.com/vadimdemedes/ink could be a replacement for vorpal

@lukeed
Copy link

lukeed commented Jul 17, 2019

Ink is good. I forgot to mention before that sade is also an option for your CLI builder. Its only dependency is mri, which is already in this list

@joshmanders
Copy link

Hello, we use Moleculer at my company and vorpal's vulnerabilities are a concern to us and we'd like to get this resolved. What can I do to help with this? I can fork and start a PR to replace vorpal with something else.

Thanks.

@icebob
Copy link
Member Author

icebob commented Jul 18, 2019

vorpal is more than the ink. It has a REPL mode with command & argument handling and autocomplete feature. Ink (as soon as I can see) is a react based GUI for console apps.
The better option would be to fix vulnerable packages in vorpal. I've created a fork here

@joshmanders
Copy link

Yeah I was looking over alternatives and none really seem to do what moleculer uses vorpal for.

@icebob do you plan to utilize the fork in moleculer-repl by publishing a package or directly to the github repo in the package.json? I can start working on getting the vulnerabilities patched and submit a PR to the fork.

@tinchoz49
Copy link
Contributor

tinchoz49 commented Jul 18, 2019

This is another one more close to the vorpal concept: https://github.com/drew-y/cliffy

But I agree:

The better option would be to fix vulnerable packages in vorpal. I've created a fork here

@icebob
Copy link
Member Author

icebob commented Jul 18, 2019

@joshmanders good news. It would be good if you could fix it. I plan to publish the fixed vorpal under the org like: @moleculer/vorpal

@icebob
Copy link
Member Author

icebob commented Jul 18, 2019

@tinchoz49 great, yes, cliffy can be an alternative. Btw, I've starred it for a while :P

@joshmanders
Copy link

@joshmanders good news. It would be good if you could fix it. I plan to publish the fixed vorpal under the org like: @moleculer/vorpal

Excellent. I have a PR waiting right now, just have noticed some tests are timing out so I'm trying to figure that out.

@GrosSacASac
Copy link

I heard that bluebird is no longer needed

@abdavid
Copy link

abdavid commented May 20, 2020

@icebob Could we add lodash to the list of libraries that Moleculer could live without? I dissected the framework a while back while playing around with making it a first-class typescript citizen and noticed a lot of lodash usage that could be replaced by simple native js. The reason I particularly am mentioning this lib is because of the bad pratices like prototype pollution it often entails.

https://github.com/moleculerjs/moleculer/search?q=isObject&unscoped_q=isObject

@abdavid
Copy link

abdavid commented May 20, 2020

Latest tree btw

moleculer@0.14.6 /Users/davborre/Source/moleculer
├─┬ args@5.0.1
│ ├── camelcase@5.0.0
│ ├─┬ chalk@2.4.2
│ │ ├─┬ ansi-styles@3.2.1
│ │ │ └─┬ color-convert@1.9.1
│ │ │   └── color-name@1.1.3
│ │ ├── escape-string-regexp@1.0.5
│ │ └─┬ supports-color@5.4.0
│ │   └── has-flag@3.0.0
│ ├── leven@2.1.0
│ └── mri@1.1.4
├── es6-error@4.1.1
├── eventemitter2@6.4.0
├── fastest-validator@1.4.1
├── fn-args@5.0.0
├─┬ glob@7.1.6
│ ├── fs.realpath@1.0.0
│ ├─┬ inflight@1.0.6
│ │ ├── once@1.4.0 deduped
│ │ └── wrappy@1.0.2
│ ├── inherits@2.0.3
│ ├─┬ minimatch@3.0.4
│ │ └─┬ brace-expansion@1.1.11
│ │   ├── balanced-match@1.0.0
│ │   └── concat-map@0.0.1
│ ├─┬ once@1.4.0
│ │ └── wrappy@1.0.2 deduped
│ └── path-is-absolute@1.0.1
├── ipaddr.js@1.9.1
├── kleur@3.0.3
├── lodash@4.17.15
├─┬ lru-cache@5.1.1
│ └── yallist@3.0.3
└── node-fetch@2.6.0

@icebob
Copy link
Member Author

icebob commented May 21, 2020

Yes, it would be good to drop lodash lib from the core repo. The most important method is the _.defaultsDeep. It is used in many places, so it's the most important to find an exact same implementation or copy out from the lodash.

@joshmanders
Copy link

Yes, it would be good to drop lodash lib from the core repo. The most important method is the _.defaultsDeep. It is used in many places, so it's the most important to find an exact same implementation or copy out from the lodash.

Could just depend on lodash.defaultsDeep which is just the dependencies needed for that function.

@icebob
Copy link
Member Author

icebob commented May 21, 2020

Hmm, good idea!

@icebob
Copy link
Member Author

icebob commented May 21, 2020

By the way, I've just found cac which is same as args but no dependencies.

Only need to change the glob to avoid another big branch

@abdavid
Copy link

abdavid commented May 21, 2020 via email

@lukeed
Copy link

lukeed commented May 21, 2020

cac has dependencies (including mri) – it's just all inlined into one big pre-built file, which prevents any cross-dep code reuse

@icebob
Copy link
Member Author

icebob commented May 21, 2020

Ohh, thanks @lukeed, I didn't see it.

@lukeed
Copy link

lukeed commented May 21, 2020

It's actually only mri - I thought there were more. This means cac basically the same as sade...good upgrade either way

@icebob
Copy link
Member Author

icebob commented May 22, 2020

@abdavid Here is a list about the used Lodash methods:
image

@abdavid
Copy link

abdavid commented May 22, 2020 via email

@abdavid
Copy link

abdavid commented May 22, 2020

@icebob Do we have any docs / guides on performance testing? Would be necessary to have before and after measurements to ensure we keep things speedy

@icebob
Copy link
Member Author

icebob commented May 22, 2020

Yes, you can with npm run bench. It executes common performance benchmark tests, so you can measure before & after with it.

@Wallacy
Copy link
Contributor

Wallacy commented May 26, 2020

@icebob Another option to replace Vorpal: https://github.com/f/omelette

Has zero dependencies;

@icebob
Copy link
Member Author

icebob commented May 27, 2020

Thanks @Wallacy but I think it's not a replacement of vorpal. It's just a part of it.

@AndreMaz
Copy link
Member

AndreMaz commented Jun 8, 2020

One possible replacement for vorpal in moleculer-repl is shargs.

Some features:

  • Highly customizable command-line arguments parser and usage documentation generator.
  • 35+ opt-in features, e.g. (multiple) subcommands, spelling mistake detection, default values, and (best guess) casting.
  • Synchronous and Promise-based asynchronous mode with async/await support.
  • Automatic usage documentation generation with fine-grained control over layouts and styles.
  • Easily extensible with your own custom parser stages and custom usage layouts.
  • Extensively documented and very well tested (750+ unit and integration tests).
  • Modular library layout with zero runtime dependencies.

and @Yord is currently working on REPL mode and autocompletion

@icebob
Copy link
Member Author

icebob commented Jun 9, 2020

It sounds great. it has no dependency.

@Yord
Copy link

Yord commented Jun 9, 2020

Hey all, shargs is meant to replace the command-line parser of another project of mine because it had way too many dependencies (27!). Seems our causes are aligned.

The REPL mode is still very fresh, but I am interested in spending more time.

@icebob
Copy link
Member Author

icebob commented Jun 9, 2020

@Yord Do you know vorpal? Are there any other missing parts in shargs? We are using it in our moleculer-repl

@Yord
Copy link

Yord commented Jun 9, 2020

@icebob I have never worked with vorpal.

The command-line parser parts of shargs are almost at 1.0.0, error message ergonomy is left (and bash autocomplete).

As for REPLs, shargs has basic REPL functionality based on the Node REPL and autocomplete for commands. These two parts are not tested, yet.

I could imagine shargs REPL misses text colors and formatting (aka tables) unless the action output does that.

Is there anything apparent to vorpal users that I miss?

Edited to add: Shargs REPL misses pipes and prompts.

@AndreMaz
Copy link
Member

AndreMaz commented Jun 9, 2020

I could imagine shargs REPL misses text colors and formatting (aka tables) unless the action output does that.

You are right, the coloring and formatting are done directly in action (e.g., service )

Edited to add: Shargs REPL misses pipes and prompts.

I think that we don't use either of those features. We only use "a small portion" of vorpal

@icebob
Copy link
Member Author

icebob commented Jun 9, 2020

Would be good to create a branch in moleculer-repl and trying to add Shargs instead of vorpal. @AndreMaz do you have some capacity to work on it? Maybe @Yord can help if something is missing :)

@Yord
Copy link

Yord commented Jun 9, 2020

@icebob, @AndreMaz I will gladly ship in if I can be of help.

@AndreMaz
Copy link
Member

AndreMaz commented Jun 9, 2020

@icebob I will look at it during weekend. In theory it shouldn't be very complex. I just need to find the equivalent for vorpal's syntax. E.g.:

vorpal
  .command('foo <requiredArg> [optionalArg]')
  .autocomplete({
    data() {
        return // Return autocomplete array
  })
  .option('-v, --verbose', 'Print foobar instead.')
  .description('Outputs "bar".')
  .alias('foosball')
  .action(function(args, callback) {
    if (args.options.verbose) {
      this.log('foobar');
    } else {
      this.log('bar');
    }
    callback();
  });

@Yord do you have any tutorial for newbies? Your docs are very good but I didn't found any introductory tutorial.

@Yord
Copy link

Yord commented Jun 9, 2020

@AndreMaz Fortunately I wrote one just yesterday:

https://github.com/Yord/shargs-tutorial-git/blob/master/README.md

@AndreMaz
Copy link
Member

AndreMaz commented Jun 9, 2020

Sweet 😄 Thanks

@Yord
Copy link

Yord commented Jun 13, 2020

@AndreMaz Just wanted to let you know I have incorporated recent updates into shargs-example-repl.

To elaborate on this: The API has only slight changes, the updates concern mainly autocomplete internals.

@AndreMaz
Copy link
Member

@Yord thanks for the info. Can you add the changelog to your repos? It helps to track the changes

@Yord
Copy link

Yord commented Jun 14, 2020

@AndreMaz Yes, I will do that. Sorry for the sloppy work. I usually start a changelog with the first stable version, but here, it definitely makes sense to start right now.

Changelogs:

@gautaz
Copy link
Contributor

gautaz commented Jan 15, 2023

Hello, sorry to barge in but I think that what I raised here might be related to this issue.

My main concern is related to the fact that some of us (I have no idea how many) are using the Moleculer broker without the runner.
I was wondering if moving the broker to something like moleculer-core (or the runner to moleculer-runner) would be an acceptable solution.

The runner is great but it also comes with its cost in term of dependencies and I guess that part of the dependencies raised in this issue is in fact due to the runner (another batch is certainly due to the REPL).
⚠️ This last one is an assumption, I did no check the source code so I may be totally wrong (which would be far from being the first time 😆).

@icebob
Copy link
Member Author

icebob commented Jan 16, 2023

I plan to split this repo to multiple packages in v1.0 and release them under @moleculer/....

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

No branches or pull requests

10 participants