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

Replace inquirer with enquirer #12305

Closed
takenspc opened this issue Mar 29, 2021 · 7 comments
Closed

Replace inquirer with enquirer #12305

takenspc opened this issue Mar 29, 2021 · 7 comments

Comments

@takenspc
Copy link
Contributor

The package install size of lighthouse weighs 65.6MB.

https://packagephobia.com/result?p=lighthouse

This seems big. Though some packages are unavoidable, some packages can be replaced with smaller one.

The inquirer is one of big packages as it depends on rxjs. Replacing inquirer with enquirer would reduce the install size.

  • inquirer: install size
  • enquirer: install size

Pros:

  • We can reduce the install size

Cons:

  • enquirer's type definitions are not suitable for lighthouse usage so that we need our own type definitions

Note: upgrading jsonld to 5 will reduce the install size too.

  • jsonld@4.0.1: install size
  • jsonld@5.0.0: install size
@takenspc
Copy link
Contributor Author

Following is the proof-of-concept changes.
takenspc/lighthouse@master...use-enquirer

Though a few devDependencies still depends on inquirer, my motivation is speeding up CI setup which runs lighthouse so that not installing inquirer by executing npm i lighthouse is fine.

@connorjclark
Copy link
Collaborator

history can be found: https://github.com/GoogleChrome/lighthouse/issues?q=inquirer+is%3Aclosed

#642 (comment) heh :P

@takenspc would you like to send a PR?

image

nice


RE: the lack of types for Confirm enquirer/enquirer#135 (comment) , Could you try making ./types/enquirer.d.ts and defining the Confirm function there?

@brendankenny
Copy link
Member

See also this comment on getting matching behavior in enquirer, but that was more than two years ago so maybe easier to fix now :)

#6658 (comment)

@brendankenny
Copy link
Member

Note: upgrading jsonld to 5 will reduce the install size too.

  • jsonld@4.0.1: install size
  • jsonld@5.0.0: install size

It would be great to update this since we just updated to 4.0.1 and it looks like most of the changes are dropping request and node <12 support, but we'll have to wait for digitalbazaar/jsonld.js#443 and the upstream digitalbazaar/http-client#6 to be resolved (currently the used version of a transitive dep has an engines: {node: "^10.17.0"} entry in their package.json so yarn refuses to install on any node 11+).

@davidlehn
Copy link

jsonld@5.2.0 has the updated @digitalbazaar/http-client@1.1.0, and also an updated rdf-canonize@3.0.0 which trims off some size due to dropping shipped browser bundles.

@brendankenny
Copy link
Member

oh nice, thanks for sharing the update!

takenspc added a commit to takenspc/lighthouse that referenced this issue Apr 8, 2021
takenspc added a commit to takenspc/lighthouse that referenced this issue Apr 8, 2021
@brendankenny
Copy link
Member

both of these were updated. Thanks again @takenspc!

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

6 participants