Skip to content

cannot use unique symbol as object key when generating declarations from JS #37022

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
boneskull opened this issue Feb 25, 2020 · 2 comments · Fixed by #37444
Closed

cannot use unique symbol as object key when generating declarations from JS #37022

boneskull opened this issue Feb 25, 2020 · 2 comments · Fixed by #37444
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@boneskull
Copy link
Contributor

TypeScript Version: 3.9.0-dev.20200225

Also tried in 3.8.2.

Search Terms: js symbol declaration ts9006

Code

See gist https://gist.github.com/boneskull/93484ca75cef49c11c69b1d416153bc4

Additionally, a.d.ts is created successfully:

export const kSymbol: unique symbol;
export type WithSymbol = {
    [kSymbol]: true;
};

I could not find a workaround (though there may be one!), since the unique symbol syntax is seemingly unavailable to JavaScript; it's not allowed in a @type docstring attribute. A workaround like this was shown in #36976.

Expected behavior:

b.d.ts is generated without error.

Actual behavior:

kSymbol is considered private, b.d.ts is not generated, and an error is emitted:

b.js:1:1 - error TS9006: Declaration emit for this file requires using private name 'kSymbol' from module '"/path/to/a"'. An explicit type annotation may unblock declaration emit.

1 import {kSymbol} from './a';
  ~~~~~~


Found 1 error.

Note that kSymbol is explicitly exported, and is not "private".

Playground Link: (playgrounds don't support generating declarations)

Related Issues: This may be related to #36976.

@boneskull
Copy link
Contributor Author

FWIW, the "An explicit type annotation may unblock declaration emit." needs a better explanation. An explicit type annotation where?

boneskull added a commit to IBM/report-toolkit that referenced this issue Feb 25, 2020
boneskull added a commit to IBM/report-toolkit that referenced this issue Feb 25, 2020
@tadhgmister
Copy link

tadhgmister commented Feb 29, 2020

A possible root of the problem seems to be that import("./a").WithSymbol seems to try to copy the type definition in place (which it can't because it needs to be able to refer to kSymbol in a value context) Instead of just emitting the import type directly like it does for for classes.

For example if our a.js looked like this:

export const kSymbol = Symbol("ksymbol");
/** @typedef {{[kSymbol]: boolean}} WithSymbol */

/** @class */
export function Foo() {
    this.a = 1;
    this.b = 2;
}
/**  @typedef {{a: number, b: number}} Bar */

And our b.js looked like this:

/**
 * @param {import('./a').Foo} value
 */
function useFoo(value) {}
/**
 * @param {import('./a').Bar} value
 */
function useBar(value) {}

Then the emitted code for b.d.ts doesn't just compile to import both but instead copies the typedef in place:

/**
 * @param {import('./a').Foo} value
 */
declare function useFoo(value: import("./a").Foo): void;
/**
 * @param {import('./a').Bar} value
 */
declare function useBar(value: {
    a: number;
    b: number;
}): void;

If we replaced this.a with this[kSymbol] the class case works fine because it is being imported directly, but having a computed property in a typedef fails because it is tried to be copied in place and needs to refer to kSymbol in a value context which it can't. Why aren't we just emitting the same import for typedefs as for classes? Like this typescript code is perfectly legal and works as intended:

declare function useWithSymbol(value: import("./a").WithSymbol) {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants