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

Only import used lodash methods #923

Merged
merged 2 commits into from
May 31, 2020
Merged

Only import used lodash methods #923

merged 2 commits into from
May 31, 2020

Conversation

merceyz
Copy link
Contributor

@merceyz merceyz commented May 27, 2020

What's the problem this PR addresses?

  • Inquirer is currently loading lodash through the main entrypoint which makes it impossible for webpack to treeshake the unused code.
  • Yarn is using Inquirer in its cli and, because of this, is shipping a lot of unnecessary code that we don't need in our bundle.

Compared to the other PRs that attempted to tackle this in the past (https://github.com/SBoudrias/Inquirer.js/pulls?q=is%3Apr+is%3Aclosed+lodash+module) this PR doesn't use the per method packages (they're deprecated and didn't help; https://lodash.com/per-method-packages) but instead requires the used methods directly. This will get rid of unused code and potentially help startup times as there is less code to process.

I made the diff as small as possible which is why I construct a _ object with the used methods. If the project switches to es2015 imports then this can be removed in favour of using https://github.com/lodash/babel-plugin-lodash

How did you fix it?

Imported only the used lodash methods

@LitoMore LitoMore self-requested a review May 28, 2020 02:27
@LitoMore LitoMore requested a review from SBoudrias May 29, 2020 02:30
@LitoMore LitoMore changed the title perf: only import used lodash methods Only mport used lodash methods May 31, 2020
@LitoMore LitoMore merged commit 039a55c into SBoudrias:master May 31, 2020
@LitoMore LitoMore changed the title Only mport used lodash methods Only import used lodash methods May 31, 2020
@merceyz merceyz deleted the lodash branch May 31, 2020 09:49
@merceyz
Copy link
Contributor Author

merceyz commented Jun 13, 2020

Hey @LitoMore 👋, when do you think this will be released?

@LitoMore
Copy link
Collaborator

@merceyz I don't have permission to publish on npm, let me contact @SBoudrias for help.

@LitoMore
Copy link
Collaborator

@merceyz 7.2.0 released.

jdoyle65 pushed a commit to jdoyle65/Inquirer.js that referenced this pull request Jan 19, 2021
* perf: only import used lodash methods

* fix: add missing import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants