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

Bug: private class fields transformed incorrectly in ESM #2230

Closed
dreamorosi opened this issue Mar 14, 2024 · 2 comments · Fixed by #2233
Closed

Bug: private class fields transformed incorrectly in ESM #2230

dreamorosi opened this issue Mar 14, 2024 · 2 comments · Fixed by #2233
Assignees
Labels
bug Something isn't working completed This item is complete and has been merged/shipped idempotency This item relates to the Idempotency Utility

Comments

@dreamorosi
Copy link
Contributor

Expected Behaviour

When using the Idempotency utility with a function formatted as ESM the utility and the function should work as intended.

Current Behaviour

As reported by @AllyMurray in this comment, using the Idempotency utility with ESM bundling causes the following runtime error: __classPrivateFieldSet is not defined at import time.

Looking at the stack trace, the error can be traced back to a private field in the IdempotencyConfig class. This is not the only occurrence of private fields (v8.dev), however it's the first occurrence in the import stack.

Code snippet

import { IdempotencyConfig } from '@aws-lambda-powertools/idempotency';

new IdempotencyConfig({});

Steps to Reproduce

  1. Install @aws-lambda-powertools/idempotency
  2. Bundle your function using ESM format
  3. Run the function
  4. Observe the error in the logs

Possible Solution

After a few tests it seems that the root cause for the issue is the noEmitHelpers: true setting that we have enabled for the ESM builds here and its interaction with the ES2021 (Node.js 16) target that we currently use.

When enabled, this setting makes it so that certain language features are polyfilled during transform depending on the targeted version. In the case of private class fields, depending on the target, the following code:

class MyClass {
  readonly #enabled: boolean;

  public constructor() {
    this.#enabled = true;
  }

  public getEnabled(): boolean {
    return this.#enabled;
  }
}

const foo = new MyClass();

becomes this:

var __classPrivateFieldSet = (this && this.__classPrivateFieldSet) || function (receiver, state, value, kind, f) {
    if (kind === "m") throw new TypeError("Private method is not writable");
    if (kind === "a" && !f) throw new TypeError("Private accessor was defined without a setter");
    if (typeof state === "function" ? receiver !== state || !f : !state.has(receiver)) throw new TypeError("Cannot write private member to an object whose class did not declare it");
    return (kind === "a" ? f.call(receiver, value) : f ? f.value = value : state.set(receiver, value)), value;
};
var __classPrivateFieldGet = (this && this.__classPrivateFieldGet) || function (receiver, state, kind, f) {
    if (kind === "a" && !f) throw new TypeError("Private accessor was defined without a getter");
    if (typeof state === "function" ? receiver !== state || !f : !state.has(receiver)) throw new TypeError("Cannot read private member from an object whose class did not declare it");
    return kind === "m" ? f : kind === "a" ? f.call(receiver) : f ? f.value : state.get(receiver);
};
var _MyClass_enabled;
class MyClass {
    constructor() {
        _MyClass_enabled.set(this, void 0);
        __classPrivateFieldSet(this, _MyClass_enabled, true, "f");
    }
    getEnabled() {
        return __classPrivateFieldGet(this, _MyClass_enabled, "f");
    }
}
_MyClass_enabled = new WeakMap();
const foo = new MyClass();

instead of remaining as it is.

When implementing v2 I disabled the setting for ESM builds thinking that we wouldn't need them since according to the v8.dev docs the feature is supported in Node.js 14+ and we have a target of ES2021 which roughly aligns to Node.js 16.

Apparently I was mistaken, and the combination of noEmitHelpers: true and target: 'ES2021' caused the esm bundle to be emitted like this, aka __classPrivateFieldSet is not included even though the code has been downleveled and expects it:

var _MyClass_enabled;
class MyClass {
    constructor() {
        _MyClass_enabled.set(this, void 0);
        __classPrivateFieldSet(this, _MyClass_enabled, true, "f");
    }
    getEnabled() {
        return __classPrivateFieldGet(this, _MyClass_enabled, "f");
    }
}
_MyClass_enabled = new WeakMap();
const foo = new MyClass();

The short term solution is to disable the noEmitHelpers setting and make a release, this will fix the issue at the cost of a few extra lines (~20) being present in the bundle.

In the medium term, and specifically once we drop support for Node.js 16 (#2223) in a few months, we'll change the target to ES2022 and when that happens we will be able to disable noEmitHelpers again since TypeScript emits the private fields as they are (aka without needing an helper fn).

Additionally, and this is just as critical, we'll have to prioritize adding new tests that check ESM builds and catch these types of issues.

Powertools for AWS Lambda (TypeScript) version

latest

AWS Lambda function runtime

20.x

Packaging format used

npm

Execution logs

No response

@dreamorosi dreamorosi added bug Something isn't working idempotency This item relates to the Idempotency Utility confirmed The scope is clear, ready for implementation labels Mar 14, 2024
@dreamorosi dreamorosi self-assigned this Mar 14, 2024
@dreamorosi dreamorosi linked a pull request Mar 14, 2024 that will close this issue
9 tasks
Copy link
Contributor

⚠️ COMMENT VISIBILITY WARNING ⚠️

This issue is now closed. Please be mindful that future comments 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.

@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 Mar 15, 2024
Copy link
Contributor

This is now released under v2.0.3 version!

@github-actions github-actions bot added completed This item is complete and has been merged/shipped and removed pending-release This item has been merged and will be released soon labels Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working completed This item is complete and has been merged/shipped idempotency This item relates to the Idempotency Utility
Projects
Development

Successfully merging a pull request may close this issue.

1 participant