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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃殌 Feature: Don't inline if doing so would increase code size #3

Closed
3 tasks done
JoshuaKGoldberg opened this issue Oct 4, 2022 · 2 comments 路 Fixed by #178
Closed
3 tasks done

馃殌 Feature: Don't inline if doing so would increase code size #3

JoshuaKGoldberg opened this issue Oct 4, 2022 · 2 comments 路 Fixed by #178
Labels
status: accepting prs Please, send a pull request to resolve this! 馃檹 type: feature New enhancement or request 馃殌

Comments

@JoshuaKGoldberg
Copy link
Owner

Bug Report Checklist

  • I have tried restarting my IDE and the issue persists.
  • I have pulled the latest main branch of the repository.
  • I have searched for related issues and found none that matched my issue.

Overview

For projects that are using ts-function-inliner primarily to reduce code size, it may be preferable to skip inlining functions if the replaced call would actually result in more output code. Especially if the function can't be removed (e.g. it's exported) and/or is called in many places.

Let's add an option to skip inlining a function in that case. I'm not sure what it should be called: maybe, skipLargerReplacements: true?

Additional Info

For an example, take this function and two calls to it:

function shortName(a: number) {
  return a + 1.000000000000000000002;
}

shortName(3);
shortName(4);

Code block character length: 100

By default, ts-function-inliner would produce:

function shortName(a: number) {
  return a + 1.000000000000000000002;
}

3 + 1.000000000000000000002;
4 + 1.000000000000000000002;

Code block character length: 130

@JoshuaKGoldberg JoshuaKGoldberg added type: feature New enhancement or request 馃殌 status: accepting prs Please, send a pull request to resolve this! 馃檹 labels Oct 4, 2022
@JoshuaKGoldberg
Copy link
Owner Author

Hmm, I can't think of any case where someone wouldn't want this behavior enabled. I'm going to go ahead and make it always on. If someone wants an option added later, they can file a followup issue.

@JoshuaKGoldberg JoshuaKGoldberg changed the title 馃殌 Feature: Add an option to not inline if doing so would increase code size 馃殌 Feature: Don't inline if doing so would increase code size Jun 3, 2024
JoshuaKGoldberg added a commit that referenced this issue Jun 3, 2024
## PR Checklist

- [x] Addresses an existing open issue: fixes #3
- [x] That issue was marked as [`status: accepting
prs`](https://github.com/JoshuaKGoldberg/ts-function-inliner/issues?q=is%3Aopen+is%3Aissue+label%3A%22status%3A+accepting+prs%22)
- [x] Steps in
[CONTRIBUTING.md](https://github.com/JoshuaKGoldberg/ts-function-inliner/blob/main/.github/CONTRIBUTING.md)
were taken

## Overview

Per the linked issue, this stops the transform from _increasing_ text
size. Long functions will no longer be inlined.

Also adds support for function expressions in variables, as that came up
in threshold testing.
Copy link

github-actions bot commented Jun 3, 2024

馃帀 This is included in version v0.2.0 馃帀

The release is available on:

Cheers! 馃摝馃殌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Please, send a pull request to resolve this! 馃檹 type: feature New enhancement or request 馃殌
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant