Skip to content

Using object destructuring with ECMAScript's private field as computed property name leads to runtime error #37791

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
adamgauthier opened this issue Apr 4, 2020 · 8 comments · Fixed by #38135
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@adamgauthier
Copy link

TypeScript Version: 3.8.3

Search Terms: computed object property name destructuring private ECMAScript

Code

class PropertyAccessor {
    readonly #propertyName: string;

    constructor(propertyName: string) {
        this.#propertyName = propertyName;
    }

    getPropertyValue(obj: Record<string, string | undefined>): string | undefined {
        const { [this.#propertyName]: value } = obj;
        return value;
    }
}

const accessor = new PropertyAccessor('name');
console.log(
    accessor.getPropertyValue({ name: 'Adam' })
);

Expected behavior:
'Adam' is logged at runtime.

Actual behavior:
Runtime error occurs:

Private field '#propertyName' must be declared in an enclosing class 

Additional information:
I just started using TypeScript and I ran into this problem while converting one of my javascript projects, the behavior was confusing to me. It seems like the code is transpiled incorrectly, as it works when not using destructuring syntax:

const { [this.#propertyName]: value } = obj;
// compiles to
const { [(_propertyName = new WeakMap(), this.#propertyName)]: value } = obj;

const value = obj[this.#propertyName];
// compiles to
const value = obj[__classPrivateFieldGet(this, _propertyName)];

The error does not occur when using TypeScript's private field instead of ECMAScript's.
The error does not occur when targeting ESNext.

Related Issues: Maybe #26572 #26355? Doesn't seem like the same problem but similar keywords.

Output
"use strict";
var __classPrivateFieldSet = (this && this.__classPrivateFieldSet) || function (receiver, privateMap, value) {
    if (!privateMap.has(receiver)) {
        throw new TypeError("attempted to set private field on non-instance");
    }
    privateMap.set(receiver, value);
    return value;
};
var __classPrivateFieldGet = (this && this.__classPrivateFieldGet) || function (receiver, privateMap) {
    if (!privateMap.has(receiver)) {
        throw new TypeError("attempted to get private field on non-instance");
    }
    return privateMap.get(receiver);
};
var _propertyName;
class PropertyAccessor {
    constructor(propertyName) {
        _propertyName.set(this, void 0);
        __classPrivateFieldSet(this, _propertyName, propertyName);
    }
    getPropertyValue(obj) {
        const { [(_propertyName = new WeakMap(), this.#propertyName)]: value } = obj;
        return value;
    }
}
const accessor = new PropertyAccessor('name');
console.log(accessor.getPropertyValue({ name: 'Adam' }));
Compiler Options
{
  "compilerOptions": {
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "alwaysStrict": true,
    "esModuleInterop": true,
    "declaration": true,
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "moduleResolution": 2,
    "target": "ES2017",
    "jsx": "React",
    "module": "ESNext"
  }
}

Playground Link: Provided

@a-tarasyuk
Copy link
Contributor

a-tarasyuk commented Apr 22, 2020

The following example works for the ESNext target, because of TS emits JavaScript code without polyfills for private fields

"use strict";
class Foo {
    constructor(value) {
        this.#value = value;
    }
    #value; // <- declarated private value. if remove it JavaScript engines will throw 
            // Private field '#propertyName' must be declared in an enclosing class 

    getValue(data) {
        const { [this.#value]: value } = data;
        return value;
    }
}

You can try to use workaround - Playground, like so

const key = this.#value;
const { [key]: value } = data;

However, I think, it is a bug, because, the following code

const { [this.#value]: value } = data;

should be converted something like so

const { [__classPrivateFieldGet(this, _value)]: value } = data;

cc @RyanCavanaugh

@nicolo-ribaudo
Copy link

I was going to open a new issue, but I think it's the same as this one. However, I'm using computed class methods and not destructuring:

let getX: (a: A) => number;

class A {
  #x = 2;

  [(getX = (obj: A) => obj.#x, "_")]() {}
}

console.log(getX!(new A));

playground

@rbuckton
Copy link
Contributor

@nicolo-ribaudo The fix @a-tarasyuk has proposed doesn't address this, but I think I found the cause of the issue you're seeing. I've brought up the problem in the PR so hopefully we can resolve both at the same time.

@rbuckton
Copy link
Contributor

@nicolo-ribaudo I've turned your code sample into a repro the bot can track:

// @showEmit
// @target: ES2015
let getX: (a: A) => number;

class A {
  #x = 2;

  [(getX = (obj: A) => obj.#x, "_")]() {}
}

getX!(new A);

Workbench Repro

@typescript-bot typescript-bot added the Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros label Dec 18, 2020
@rbuckton
Copy link
Contributor

Same for the repro mentioned by @a-tarasyuk:

// @showEmit
// @target: ES2015
class Foo {
    #value: any;
    constructor(value: any) {
        this.#value = value;
    }
    getValue(data: any) {
        const { [this.#value]: value } = data;
        return value;
    }
}

Workbench Repro

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 18, 2020

👋 Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of the 2 repros in this issue running against the nightly TypeScript. If something changes, I will post a new comment.


Comment by @rbuckton

👍 Compiled
Emit:

"use strict";
var __classPrivateFieldSet = (this && this.__classPrivateFieldSet) || function (receiver, privateMap, value) {
    if (!privateMap.has(receiver)) {
        throw new TypeError("attempted to set private field on non-instance");
    }
    privateMap.set(receiver, value);
    return value;
};
var __classPrivateFieldGet = (this && this.__classPrivateFieldGet) || function (receiver, privateMap) {
    if (!privateMap.has(receiver)) {
        throw new TypeError("attempted to get private field on non-instance");
    }
    return privateMap.get(receiver);
};
var _value;
class Foo {
    constructor(value) {
        _value.set(this, void 0);
        __classPrivateFieldSet(this, _value, value);
    }
    getValue(data) {
        const { [(_value = new WeakMap(), this.#value)]: value } = data;
        return value;
    }
}

Comment by @rbuckton

👍 Compiled
Emit:

"use strict";
var __classPrivateFieldGet = (this && this.__classPrivateFieldGet) || function (receiver, privateMap) {
    if (!privateMap.has(receiver)) {
        throw new TypeError("attempted to get private field on non-instance");
    }
    return privateMap.get(receiver);
};
var _x;
let getX;
class A {
    constructor() {
        _x.set(this, 2);
    }
    [(_x = new WeakMap(), (getX = (obj) => obj.#x, "_"))]() { }
}
getX(new A);

Historical Information

Comment by @louistio

Version Reproduction Outputs
3.8.2, 3.9.2, 4.0.2, 4.1.2, Nightly

👍 Compiled
Emit:

"use strict";
var __classPrivateFieldSet = (this && this.__classPrivateFieldSet) || function (receiver, privateMap, value) {
    if (!privateMap.has(receiver)) {
        throw new TypeError("attempted to set private field on non-instance");
    }
    privateMap.set(receiver, value);
    return value;
};
var __classPrivateFieldGet = (this && this.__classPrivateFieldGet) || function (receiver, privateMap) {
    if (!privateMap.has(receiver)) {
        throw new TypeError("attempted to get private field on non-instance");
    }
    return privateMap.get(receiver);
};
var _value;
class Foo {
    constructor(value) {
        _value.set(this, void 0);
        __classPrivateFieldSet(this, _value, value);
    }
    getValue(data) {
        const { [(_value = new WeakMap(), this.#value)]: value } = data;
        return value;
    }
}

3.7.5

❌ Failed: -

  • Cannot redeclare block-scoped variable 'value'.
  • Cannot redeclare block-scoped variable 'value'.
  • Invalid character.
  • Invalid character.
  • Invalid character.
  • ',' expected.
  • Property destructuring pattern expected.

Emit:
"use strict";
class Foo {
    constructor(value) {
        this.;
        value = value;
    }
    getValue(data) {
        const { [this.]: , value, value } = data;
        return value;
    }
}

Comment by @louistio

Version Reproduction Outputs
3.8.2, 3.9.2, 4.0.2, 4.1.2, Nightly

👍 Compiled
Emit:

"use strict";
var __classPrivateFieldGet = (this && this.__classPrivateFieldGet) || function (receiver, privateMap) {
    if (!privateMap.has(receiver)) {
        throw new TypeError("attempted to get private field on non-instance");
    }
    return privateMap.get(receiver);
};
var _x;
let getX;
class A {
    constructor() {
        _x.set(this, 2);
    }
    [(_x = new WeakMap(), (getX = (obj) => obj.#x, "_"))]() { }
}
getX(new A);

3.7.5

❌ Failed: -

  • A computed property name must be of type 'string', 'number', 'symbol', or 'any'.
  • Member '[(getX = (obj: A) => obj.' implicitly has an 'any' type.
  • Duplicate identifier 'x'.
  • Property 'x' has no initializer and is not definitely assigned in the constructor.
  • Subsequent property declarations must have the same type. Property 'x' must be of type 'number', but here has type 'any'.
  • Member '"_"' implicitly has an 'any' type.
  • Invalid character.
  • Invalid character.
  • ';' expected.
  • ';' expected.
  • Unexpected token. A constructor, method, accessor, or property was expected.
  • Unexpected token. A constructor, method, accessor, or property was expected.
  • '=>' expected.
  • Declaration or statement expected.

Emit:
"use strict";
let getX;
class A {
    constructor() {
        this.x = 2;
    }
}
(getX = (obj) => obj.);
() => { };
getX(new A);

@orta
Copy link
Contributor

orta commented Dec 18, 2020

@typescript-bot run repros

@typescript-bot
Copy link
Collaborator

Heya @orta, I've started to run the code sample repros for you. Here's the link to my best guess at the log.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
7 participants