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

perf: reduce bundle size #4315

Merged
merged 2 commits into from
Apr 7, 2022
Merged

perf: reduce bundle size #4315

merged 2 commits into from
Apr 7, 2022

Conversation

merceyz
Copy link
Member

@merceyz merceyz commented Apr 1, 2022

What's the problem this PR addresses?

The bundle size increased from 1 973 101 bytes to 3 282 027 bytes in #4253.

How did you fix improve it?

  • Patch ink to only import the lodash method it needs.
  • Define some process.env.* values so ESBuild can do some more tree shaking.

Results

Size in bytes
Master 3 288 047
Patch ink 3 209 390
Define process.env.NODE_ENV 3 004 912
Define process.env.DEV 2 794 423
Define process.env.TEST_ENV 2 794 247
Define misc. mkdirp test variables 2 794 130
YARN_IGNORE_PATH=1 hyperfine -w 5 "node ./master.cjs --version" "node ./after.cjs --version"
Benchmark 1: node ./master.cjs --version
  Time (mean ± σ):     218.1 ms ±   1.3 ms    [User: 227.8 ms, System: 22.2 ms]
  Range (min … max):   215.2 ms … 219.6 ms    13 runs

Benchmark 2: node ./after.cjs --version
  Time (mean ± σ):     207.6 ms ±   1.6 ms    [User: 227.4 ms, System: 12.2 ms]
  Range (min … max):   205.0 ms … 210.0 ms    14 runs

Summary
  'node ./after.cjs --version' ran
    1.05 ± 0.01 times faster than 'node ./master.cjs --version'

Checklist

  • I have read the Contributing Guide.
  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

RDIL
RDIL previously approved these changes Apr 2, 2022
@trivikr
Copy link
Contributor

trivikr commented Apr 4, 2022

  • Patch ink to only import the lodash method it needs

Is there an issue or PR upstream for merging this fix?

I see that the source code is already doing named imports, probably some issue with transpilation
https://github.com/vadimdemedes/ink/blob/4fa00de3a0e8b96562c85d3b65673be0dc15ffc2/src/ink.tsx#L2

@merceyz
Copy link
Member Author

merceyz commented Apr 6, 2022

Is there an issue or PR upstream for merging this fix?

There is now vadimdemedes/ink#511

I see that the source code is already doing named imports, probably some issue with transpilation

That doesn't matter in this case as lodash isn't tree-shakeable.

@arcanis arcanis merged commit a50d10c into master Apr 7, 2022
@arcanis arcanis deleted the merceyz/perf/bundle-size branch April 7, 2022 20:06
merceyz added a commit that referenced this pull request Oct 22, 2023
Co-authored-by: Maël Nison <nison.mael@gmail.com>
(cherry picked from commit a50d10c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants