search

frctl / fractal

A tool to help you build and document website component libraries and design systems.
https://fractal.build
MIT License
2.11k stars 168 forks source link

Migrate away from vorpal #671

Open mihkeleidast opened 3 years ago

mihkeleidast commented 3 years ago

What problem would this feature solve?

Vorpal has not been updated in four years, its dependencies are out of date and give our users vulnerability notices when installing Fractal.

This should also fix a deprecation warning users get when installing Fractal:

npm WARN deprecated core-js@2.6.11: core-js@<3 is no longer maintained and not recommended for usage due to the number of issues. Please, upgrade your dependencies to the actual version of core-js@3.

└─┬ @frctl/fractal@1.4.1
  └─┬ vorpal@1.12.0
    └─┬ babel-polyfill@6.26.0
      ├─┬ babel-runtime@6.26.0
      │ └── core-js@2.6.11  deduped
      └── core-js@2.6.11

What the feature should look like?

Fractal's CLI module should be updated to use something other than vorpal.

LeBenLeBen commented 3 years ago

What you think would be a great replacement? Inquirer.js?

mihkeleidast commented 3 years ago

By the looks of it, inquirer is not meant for building CLIs. Commander.js or yargs should be sufficient for the CLI purposes, while using inquirer for the interactive stuff when needed. I don't think neither commander or yargs provide real interactivity on the CLI...

Since vorpal is based on commander+inquirer, it would make sense (less code changes, i presume) to use those.

mihkeleidast commented 3 years ago

I'm currently looking at https://github.com/hongaar/bandersnatch as a replacement.

Chapabu commented 3 years ago

Hey all! I've got my CLI branch still kicking around, would you like me to resurrect it and carry on with this one? I also have a bunch of TypeScript stuff in there, but that's likely out of date now with the release of 4.x, but should be simple to get it updated.

I started moving it along with Commander purely because it's the most popular. Bandersnatch looks really cool, but 60 weekly downloads vs 50.5m worries me a little.

mihkeleidast commented 3 years ago

Commander is sure more popular but I think it lacks the interactive CLI part that we currently have from Vorpal. Bandersnatch is based on yargs/enquirer (vs our current commander/inquirer dependency from Vorpal) and is "inspired by" Vorpal which by the looks of it should make it easier to migrate. Also Bandersnatch is written in TypeScript which should help with our eventual move.

I'd be happy to leave this issue to you, if it'll get solved in the near future :) Would be good to get some TS setup in with it, if you already got it.

Chapabu commented 3 years ago

@mihkeleidast Yeah - leave this one with me :) I'll review/update the TS stuff in here as well whilst I'm at it.

Chapabu commented 3 years ago

@mihkeleidast Did we land on anything with the new command? Are we keeping it, or are we moving to an initialiser or create-fractal-app style approach? Or both?

mihkeleidast commented 3 years ago

@mihkeleidast Did we land on anything with the new command? Are we keeping it, or are we moving to an initialiser or create-fractal-app style approach? Or both?

I'd keep it for now, we can't remove it without introducing a breaking change. But we can refactor it into something that is easily separable in the future.

joyheron commented 3 years ago

We also are using fractal in multiple projects and have been (unhappily) ignoring the log messages about the lodash vulnerability. Is there anything I could do to help create a fix for this?

mihkeleidast commented 3 years ago

Since vorpal is totally unmaintained the only solution here is to replace it completely, which is not a trivial feat :)

@Chapabu have you made any progress here? Is there something others can help with or could I maybe work on this myself?

Chapabu commented 3 years ago

@mihkeleidast I'll pick this one back up - I think I've got a local branch with a fair amount of the work in it, and it'd be good to get back into the weeds.

Chapabu commented 3 years ago

Just some progress on this. I've got the info and build commands working as part of the monorepo examples. Working on the serve command and the new command this week - although I reckon the fractal new command might be the tricky one. After that, it's just tests - so probably a week or two then this one can be checked off.

mihkeleidast commented 3 years ago

@Chapabu A little ping on this again - when do you suppose this will be ready for some initial review and testing? It's becoming quite important since we decided to postpone the major release w/o Node 10 support until this is done, so it's blocking a bunch of updates.

joyheron commented 3 years ago

@Chapabu Is there any way that I could help with developing this feature? Would love to review/test etc. any implementations.

dasginganinja commented 2 years ago

@Chapabu Pinging for an update on this for testing the functionality. Looking to get these security issues cleared up.

mihkeleidast commented 2 years ago

Just an update here: we last chatted about this in december to figure out what we should do with the current fractal new global command in the new CLI. As it's the only reason ever someone would need to install Fractal globally, it would complicate things a bit et cetera.

An idea was to move it to a separate initializer package, and leave the CLI only for local project installs. As the separation had already been filed as an issue, we moved in that direction (see #635 and #1131). Hopefully this will allow us to move forward in the near future 🤞

joyheron commented 2 years ago

I think the idea of extracting this into a separate initializer package is a good one. This is something that is usually only necessary the first time that someone sets up a project. For my specific use cases, I rarely use the fractal new command personally, because I prefer copying my fractal configuration from project to project and setting up the directories the way that I like them.

I'm also very willing to help with testing and reviews. We do have fractal as a dependency for some client projects, and it is difficult to explain to the customer why we have issues with npm audit, so it would be of great value to me if this could be solved. I'm willing to help in any way I can. 😄

rediris commented 2 years ago

You can see we've been prodding dthree to update vorpal here: https://github.com/dthree/vorpal/pull/343#issuecomment-803110177

Chapabu commented 2 years ago

The biggest thing holding this one up from my end (outside of my time) was that the release process is a bit..well...wrong to say the least. I'm going to lump this one in with #776 (or an updated version of it) so that we can put out a major release with the breaking changes. This will allow me to completely ignore things like fractal new and the interactive CLI (that I personally have never used) and focus on the core features (build and serve).

fractal new can be replaced with an initialiser (see #1131) to leverage newer (and more common) npm features.

As discussed above, I see no reason not to use Commander, so I'll likely just reboot the work I started in 2020 :see_no_evil: .

dthree commented 1 year ago

💀 Sorry guys