FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

downgrade inquirer by bugzpodder · Pull Request #4519 · react/create-react-app · GitHub

downgrade inquirer - #4519

Closed
bugzpodder wants to merge 1 commit into
react:nextfrom
bugzpodder:package
Closed

downgrade inquirer#4519
bugzpodder wants to merge 1 commit into
react:nextfrom
bugzpodder:package

Conversation

bugzpodder commented May 25, 2018
edited
Loading

Copy link
Copy Markdown

#3880 (comment)

Basically removes rxjs dependency, which is around 18MB.
Tested yarn eject, yes/no.

iansu commented May 25, 2018

Copy link
Copy Markdown
Contributor

I'm not sure if this is the best way to handle this. If we do this then we have to remember to not update this dependency in the future. Is there a lightweight alternative to Inquirer that we could use instead?

Copy link
Copy Markdown
Author

Yep, there definitely should be, will check later. I think you added the inquirer library over prompt.js #1772
the worst case scenario is that we can bring that piece of code back?

iansu commented May 25, 2018

Copy link
Copy Markdown
Contributor

That's right. I think that was my first contribution to Create React App. 😀

I'd like to avoid going back to a custom solution. There was a reason we moved away from that in the first place. I'll look around a bit too and see if I can find a good alternative. I suppose we could also raise an issue with Inquirer if someone hasn't already.

Copy link
Copy Markdown
Author

closing this in favor of a smaller package.

bugzpodder closed this May 25, 2018
bugzpodder deleted the package branch May 25, 2018 17:56

Copy link
Copy Markdown
Author

@iansu finally found something reasonable:
If we just want Y/n prompts:
https://github.com/enquirer/prompt-confirm
If we want a more inquirer like experience:
http://enquirer.io/

Copy link
Copy Markdown
Contributor

Asking inquirer to use a different Observable implementation is probably a better bet. eslint@5 is using newer versions of inquirer, so it'll come back, anyway.

bugzpodder commented May 27, 2018
edited
Loading

Copy link
Copy Markdown
Author

hmmm. they upgraded from rx-lite (400kb) to rx5 and more recently rx6.
SBoudrias/Inquirer.js#614

iansu commented May 28, 2018

Copy link
Copy Markdown
Contributor

@bugzpodder Do you mind opening an issue with Inquirer about reducing their install size? Let's see what they say before we consider replacing the package.

Copy link
Copy Markdown
Author

Sure I'll see if i can open a few issues.
From my understanding I think they are set on using rxjs and it probably doesn't make sense for them to switch to another library. SBoudrias/Inquirer.js#614
It is a problem with RxJs itself where it is including src/ in the npm bundle. ReactiveX/rxjs#3334
I assume this will be eventually fixed by rxjs.
I also tested enquirer and its api is mostly identical to inquirer so it might be okay to just switch and deal with it later. There are a few odd UI issues that I can just submit a one-line PR for (eg showing: Do you want to eject: true instead of Do you want to eject: Yes) . but library otherwise is mostly pretty solid.

iansu commented May 28, 2018

Copy link
Copy Markdown
Contributor

It looks like RxJS is intentionally publishing src. Let's see if we can get this fixed upstream and if not then we can discuss replacing Inquirer.

bugzpodder commented May 28, 2018
edited
Loading

Copy link
Copy Markdown
Author

The src is about 2.2MB, so its unclear if excluding source will help that much.

1.7M	./operators
384K	./util
188K	./scheduler
 36K	./symbol
 96K	./testing
4.0M	./_esm5
1.5M	./add
3.9M	./_esm2015
924K	./observable
1.2M	./operator
2.4M	./bundles
2.2M	./src
 19M	.

lock Bot locked and limited conversation to collaborators Jan 19, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL