Skip to content

Feature request: consider adopting tslib for runtime helpers #1674

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

Closed
1 of 2 tasks
dreamorosi opened this issue Sep 13, 2023 · 2 comments
Closed
1 of 2 tasks

Feature request: consider adopting tslib for runtime helpers #1674

dreamorosi opened this issue Sep 13, 2023 · 2 comments
Assignees
Labels
commons This item relates to the Commons Utility feature-request This item refers to a feature request for an existing or new utility rejected This is something we will not be working on. At least, not in the measurable future
Milestone

Comments

@dreamorosi
Copy link
Contributor

Use case

When transpiling TypeScript code to JavaScript, tsc generates and injects helpers that allow developers to author code using a certain runtime (i.e. Node.js 18) and target other versions (i.e. Node.js 16).

The default behavior that TypeScript adopts is to include these helpers in each transpiled file that needs them. TypeScript however offers these same utilities also as a published npm package called tslib which can be installed and used as runtime dependency.

Following this pattern could potentially bring bundle size savings since the helpers are imported and present in the bundle only once rather than be present in each file that uses them. This however could also have some performance overhead for those customers who choose to not bundle their dependencies since there is now an additional dependency & files to read from disk.

The extent of the impact in both those areas is still to be assessed, however as a point of reference the AWS SDK for JavaScript v3 is using the tslib option suggested in this issue.

This behavior is governed by two compiler options: importHelpers and noEmitHelpers. As part of this work we should experiment with them and asses the following:

  1. see if the utilities can be compiled and run using helpers centrally imported from tslib
  2. see what impact this change brings to the final bundle size and I/O performance

As part of this issue, we could also explore two additional settings that might be adjacent to the ones above:

  • downlevelIteration
  • isolatedModules

Solution/User Experience

From customers perspective the change should be transparent and the utilities should continue working as intended without any change needed. However, in an effort to err on the safe side I suggest we make this change as part of the next major release, this way we'll have a chance to highlight the change in the migration guide/details and make it more explicit.

Alternative solutions

As an alternative we could leave things as they are and continue letting TS include these helpers in each file.

Acknowledgment

Future readers

Please react with 👍 and your use case to help us understand customer demand.

@dreamorosi dreamorosi added help-wanted We would really appreciate some support from community for this one feature-request This item refers to a feature request for an existing or new utility confirmed The scope is clear, ready for implementation commons This item relates to the Commons Utility labels Sep 13, 2023
@dreamorosi dreamorosi added this to the Version 2.0 milestone Sep 13, 2023
@dreamorosi dreamorosi removed help-wanted We would really appreciate some support from community for this one hacktoberfest labels Oct 20, 2023
@dreamorosi dreamorosi self-assigned this Oct 20, 2023
@dreamorosi dreamorosi linked a pull request Oct 20, 2023 that will close this issue
9 tasks
@dreamorosi dreamorosi removed a link to a pull request Oct 20, 2023
9 tasks
@github-actions github-actions bot added pending-release This item has been merged and will be released soon and removed confirmed The scope is clear, ready for implementation labels Oct 20, 2023
@dreamorosi dreamorosi added confirmed The scope is clear, ready for implementation and removed pending-release This item has been merged and will be released soon labels Oct 21, 2023
@dreamorosi
Copy link
Contributor Author

I have been experimenting with the importHelpers compiler option in tsconfig.json and its counterpart tslib in the project. After some consideration I am not inclined to enabled the setting and add tslib as dependency. Below my reasoning.

As stated in the OP, the importHelpers setting tells TypeScript to not generate some helpers, below the description from the TypeScript docs:

For certain downleveling operations, TypeScript uses some helper code for operations like extending class, spreading arrays or objects, and async operations. By default, these helpers are inserted into files which use them. This can result in code duplication if the same helper is used in many different modules.

When the setting is enabled, TypeScript won't emit these helpers in the transpiled code and the operator is expected to provide an implementation for the helpers and make it available at runtime. This is done via the tslib package, which is published by TypeScript and contains the same exact helpers.

If taken at face value, the idea of not emitting these helpers in each file that needs them and instead import them from a central location (node_modules) once per project makes a lot of sense.

In doing exactly this I wanted to measure the bundle size savings and so I made a build of Logger and Commons with the setting disabled in which the helpers are generated in the code (aka current status), and another with importHelpers enabled and tslib added as dependency.

Next, I created a bare bones function file that only imports the Logger utility and instantiates it:

import { Logger } from '@aws-lambda-powertools/logger';
// const { Logger } = require('@aws-lambda-powertools/logger)';

const logger = new Logger({});

The file is purposefully trivial (and short) so that we can look at the output and measure the impact that the setting has on Powertools and on functions using it.

Then, I bundled both versions with esbuild with minification, no source maps, tree shaking enabled, and mainFields set to module,main (more info here - thanks @perpil for the post). I repeated the operation for both CJS (using require import) and ESM (using import).

Below the result analyzed with this tool and using the meta file generated by esbuild as input:

CJS with tslib
image
CJS without tslib
image

ESM with & without tslib
image

Note

The ESM result is the same regardless of tslib usage because the helper is related to imports, which is not needed in ESM.

The results are pretty interesting and not at all what I was expecting. For ESM, using the setting one way or the other doesn't yield any difference (I double checked!), this is because the helpers involved in this project are related to imports and not needed in our setup.

For CJS however the difference is substantial, and the version with importHelpers: "true" and tslib as dependency is 10kb, or 27%, bigger 🤯.

I was surprised by this result as I was expecting this version to be smaller than the one with helpers emitted, so I looked more into the result.

Upon examining the lib folder in this project, I quickly noticed that in the entire project, there are only 2 occurrences of these helpers: one in the packages/logger/lib/cjs/Logger.js, and another packages/logger/lib/cjs/formatter/LogItem.js.

In both instances, this is the code emitted in each file:

var __importDefault = (this && this.__importDefault) || function (mod) {
    return (mod && mod.__esModule) ? mod : { "default": mod };
};
// ...
const lodash_merge_1 = __importDefault(require("lodash.merge"));

Looking at the unminified esbuild-bundled output in CJS I also noticed that the tslib module was not getting tree-shaken correctly and the bundle included not only the helper above but many others, which explains the hefty 10kb of imports.

With this in mind, and considering the fact that we are actually emitting these helpers only twice, it doesn't appear to be worth it to enable the importHelpers feature and add tslib at this time.

This is definitely something that we'll have to keep an eye on, since our transpiled output might change and the tradeoff could become very different, but at least for now the size of the tslib package quite literally outweighs the few helpers we generate.

As a side note, this exercise also made me think about whether there's anything that could be done about lodash.merge, which we use in a few places within the Logger utility. The fact that the official package, at least in this version, is bundled only as CJS prevents it from being tree-shaken correctly, and in general I'm not sure it's worth allocating 50% of our bundle to a single dependency. Lodash is considering dropping CJS in favor of ESM in the next major release, however at least for now there's no timeline.

For now I'll be closing this issue as not planned.

@dreamorosi dreamorosi closed this as not planned Won't fix, can't repro, duplicate, stale Oct 23, 2023
@github-project-automation github-project-automation bot moved this from Backlog to Coming soon in Powertools for AWS Lambda (TypeScript) Oct 23, 2023
@dreamorosi dreamorosi moved this from Coming soon to Closed in Powertools for AWS Lambda (TypeScript) Oct 23, 2023
@dreamorosi dreamorosi added rejected This is something we will not be working on. At least, not in the measurable future and removed confirmed The scope is clear, ready for implementation labels Oct 23, 2023
@github-actions
Copy link
Contributor

⚠️ COMMENT VISIBILITY WARNING ⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commons This item relates to the Commons Utility feature-request This item refers to a feature request for an existing or new utility rejected This is something we will not be working on. At least, not in the measurable future
Projects
Development

No branches or pull requests

1 participant