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

Setting a getter with Object.defineProperty not working within decorator #2127

Closed
InsOpDe opened this issue Aug 21, 2021 · 3 comments · Fixed by #3105
Closed

Setting a getter with Object.defineProperty not working within decorator #2127

InsOpDe opened this issue Aug 21, 2021 · 3 comments · Fixed by #3105
Assignees
Milestone

Comments

@InsOpDe
Copy link

InsOpDe commented Aug 21, 2021

Describe the bug
If I set a getter for a property with Object.defineProperty within a decorator it returns undefined instead of the variable

Input code

/* works */
test("without decorator", () => {
    const returnValue = "asdasd"
    class TestClass {
        public testProperty: Date;
    }

    Object.defineProperty(TestClass.prototype, "testProperty", {
        get: function () { return returnValue },
        enumerable: true,
        configurable: true,
    });
    const instance = new TestClass()
    expect(instance.testProperty).toBe(returnValue)
})

/* fails */
test("with decorator", () => {
    const returnValue = "asdasd"
    class TestClass2 {
        @deco public testProperty: Date;
    }
    function deco(target: any, key: string) {
        Object.defineProperty(target.constructor.prototype, key, {
            get: function () { return returnValue },
            enumerable: true,
            configurable: true,
        });
    }
    const instance = new TestClass2()
    expect(instance.testProperty).toBe(returnValue)
})

/* works */
test("with decorator without getter", () => {
    const returnValue = "asdasd"
    class TestClass2 {
        @deco public testProperty: Date;
    }
    function deco(target: any, key: string) {
        Object.defineProperty(target.constructor.prototype, key, {
            value: returnValue,
            enumerable: true,
            configurable: true,
        });
    }
    const instance = new TestClass2()
    expect(instance.testProperty).toBe(returnValue)
})

Config

/* jest.config.js*/
module.exports = {
    testEnvironment: 'node',
    transform: {
        // "^.+\\.tsx?$": "ts-jest"
		// "^.+\\.(tsx?)$": ["@swc-node/jest", {
		// 	dynamicImport: true,
		// 	emitDecoratorMetadata: true,
		// 	esModuleInterop: true,
		// 	experimentalDecorators: true,
        //     keepClassNames: true,
		// 	// module: "commonjs",
		// 	// target: "es5"
		// }]
		"^.+\\.(tsx?)$": ["@swc/jest", {
            "jsc": {
                "parser": {
                    "syntax": "typescript",
                    "decorators": true,
                    "dynamicImport": false
                },
                "transform": {
                    "legacyDecorator": true,
                    "decoratorMetadata": true
                },
                "target": "es2016",
                "loose": false,
                "externalHelpers": false,
                // Requires v1.2.50 or upper and requires target to be es2016 or upper.
                "keepClassNames": true
            }
        }]
       // "^.+\\.tsx?$": "esbuild-runner/jest"
       // "^.+\\.tsx?$": "esbuild-jest"
    }
};

Expected behavior
All tests should not fail

expect(instance.testProperty).toBe(returnValue) should return true

it works, when changing the transformer to ts-jest or esbuild-runner/jest or esbuild-jest

Version

{
    "@swc-node/jest": "^1.3.1",
    "@swc/core": "^1.2.80",
    "@swc/jest": "^0.2.2"
}

Additional context
The same problem applies to @swc-node/jest.

This issue most likely prevents users from using the known library typescript-ioc: https://github.com/thiagobustamante/typescript-ioc

thiagobustamante/typescript-ioc#68
thiagobustamante/typescript-ioc#77

@InsOpDe InsOpDe added the C-bug label Aug 21, 2021
@kdy1 kdy1 added the E-hard label Sep 29, 2021
@kdy1 kdy1 added this to the v1.2.123 milestone Dec 23, 2021
@kdy1 kdy1 self-assigned this Dec 23, 2021
@kdy1
Copy link
Member

kdy1 commented Dec 23, 2021

I tried fixing this, and it seems like babel emits the same result.

input.js

const returnValue = "asdasd"
class TestClass2 {
    @deco testProperty;
}
function deco(target, key) {
    Object.defineProperty(target.constructor.prototype, key, {
        get: function () { return returnValue },
        enumerable: true,
        configurable: true,
    });
}
const instance = new TestClass2()
console.log(instance.testProperty)

.babelrc:

{
    "plugins": [
        [
            "@babel/plugin-proposal-decorators",
            {
                "legacy": true
            }
        ],
        "@babel/plugin-proposal-class-properties"
    ]
}

Command:

npx babel src/input.js > output.js && node output.js

output.js:

var _class, _descriptor;

function _initializerDefineProperty(target, property, descriptor, context) { if (!descriptor) return; Object.defineProperty(target, property, { enumerable: descriptor.enumerable, configurable: descriptor.configurable, writable: descriptor.writable, value: descriptor.initializer ? descriptor.initializer.call(context) : void 0 }); }

function _defineProperty(obj, key, value) { if (key in obj) { Object.defineProperty(obj, key, { value: value, enumerable: true, configurable: true, writable: true }); } else { obj[key] = value; } return obj; }

function _applyDecoratedDescriptor(target, property, decorators, descriptor, context) { var desc = {}; Object.keys(descriptor).forEach(function (key) { desc[key] = descriptor[key]; }); desc.enumerable = !!desc.enumerable; desc.configurable = !!desc.configurable; if ('value' in desc || desc.initializer) { desc.writable = true; } desc = decorators.slice().reverse().reduce(function (desc, decorator) { return decorator(target, property, desc) || desc; }, desc); if (context && desc.initializer !== void 0) { desc.value = desc.initializer ? desc.initializer.call(context) : void 0; desc.initializer = undefined; } if (desc.initializer === void 0) { Object.defineProperty(target, property, desc); desc = null; } return desc; }

function _initializerWarningHelper(descriptor, context) { throw new Error('Decorating class property failed. Please ensure that ' + 'proposal-class-properties is enabled and runs after the decorators transform.'); }

const returnValue = "asdasd";
let TestClass2 = (_class = class TestClass2 {
  constructor() {
    _initializerDefineProperty(this, "testProperty", _descriptor, this);
  }

}, (_descriptor = _applyDecoratedDescriptor(_class.prototype, "testProperty", [deco], {
  configurable: true,
  enumerable: true,
  writable: true,
  initializer: null
})), _class);

function deco(target, key) {
  Object.defineProperty(target.constructor.prototype, key, {
    get: function () {
      return returnValue;
    },
    enumerable: true,
    configurable: true
  });
}

const instance = new TestClass2();
console.log(instance.testProperty);

Hmm...

@kdy1
Copy link
Member

kdy1 commented Dec 23, 2021

I investigated further.

const a = {

  get foo() {
    return 'foo'
  },
  bar: 'bar'
};


Object.defineProperty(a, 'foo', {
  configurable: true,
  enumerable: true,
  writable: true
})

Object.defineProperty(a, 'bar', {
  configurable: true,
  enumerable: true,
  writable: true
})

console.log(a)

This prints { foo: undefined, bar: 'bar' }. So the behavior of Object.defneProperty depends on the way property is declared.

kdy1 added a commit to kdy1/swc that referenced this issue Dec 23, 2021
kdy1 added a commit to kdy1/swc that referenced this issue Dec 23, 2021
kdy1 added a commit that referenced this issue Dec 23, 2021
swc_ecma_transforms_base:
 - Fix `_applyDecoratedDescriptor`. (Closes #2127)
@swc-bot
Copy link
Collaborator

swc-bot commented Oct 20, 2022

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@swc-project swc-project locked as resolved and limited conversation to collaborators Oct 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

3 participants