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

static methods omitted from 'allowJs' 'declaration' .d.ts #36270

Closed
cspotcode opened this issue Jan 17, 2020 · 1 comment · Fixed by #36274
Closed

static methods omitted from 'allowJs' 'declaration' .d.ts #36270

cspotcode opened this issue Jan 17, 2020 · 1 comment · Fixed by #36274
Assignees
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files Domain: JavaScript The issue relates to JavaScript specifically Fix Available A PR has been opened for this issue

Comments

@cspotcode
Copy link

TypeScript Version: Version 3.8.0-dev.20200117

Search Terms:

static method omitted
static method missing
allowJs function missing

Code

This is related to mochajs/mocha/issues/4154, where I'm experimenting with generating .d.ts from Mocha's JS source.

Input is JavaScript, with "allowJs" and "declaration" turned on.

module.exports = MyClass;

function MyClass() {}
MyClass.staticMethod = function() {}
MyClass.prototype.method = function() {}
MyClass.staticProperty = 123;

/**
 * Callback to be invoked when test execution is complete.
 *
 * @callback DoneCB
 * @param {number} failures - Number of failures that occurred.
 */

Expected behavior:

With "allowJs" and "declaration" enabled, emits .d.ts that includes staticMethod. I'm not sure whether it should be included in the class body or with the other exports.

export = MyClass;
declare function MyClass(): void;
declare class MyClass {
    method(): void;
    static staticMethod(): void; // <-- maybe it should be declared here?
}
declare namespace MyClass {
    export { staticMethod, staticProperty, DoneCB };
}
declare var staticMethod: () => void; // <-- maybe it should be declared here?
declare var staticProperty: number;
/**
 * Callback to be invoked when test execution is complete.
 */
type DoneCB = (failures: number) => any;

Actual behavior:

staticMethod is missing. (though it is included in the export {...} statement.)

export = MyClass;
declare function MyClass(): void;
declare class MyClass {
    method(): void;
}
declare namespace MyClass {
    export { staticMethod, staticProperty, DoneCB };
}
declare var staticProperty: number;
/**
 * Callback to be invoked when test execution is complete.
 */
type DoneCB = (failures: number) => any;

Playground Link:

I created a repl.it reproduction that calls the compiler, then prints the emitted d.ts.
https://repl.it/@AndrewBradley/missing-static-method

Related Issues:

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files Domain: JavaScript The issue relates to JavaScript specifically labels Jan 17, 2020
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 3.9.0 milestone Jan 17, 2020
@DanielRosenwasser DanielRosenwasser added Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this labels Jan 17, 2020
@weswigham
Copy link
Member

Thanks for the detailed bug report! Looks like we were failing to handle things we'd only marked with SymbolFlags.Method (vs SymbolFlags.Property or SymbolFlags.Function) in the binder during declaration generation. I have a PR up with a fix.

@weswigham weswigham removed Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this labels Jan 18, 2020
@weswigham weswigham added the Fix Available A PR has been opened for this issue label Jan 22, 2020
devtools-bot pushed a commit to ChromeDevTools/devtools-frontend that referenced this issue Feb 20, 2020
This fixes an issue where property assignments to static methods (please
don't ask me why we have them) would erroneously omit the static method
from the declaration file.

This is fixed in the latest version of TypeScript, most likely as part
of microsoft/TypeScript#36270

DISABLE_THIRD_PARTY_CHECK=Updating TypeScript

Bug: 1011811
Change-Id: I2744ef44c040fdf001f6b0be9e64a164510e0fea
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2064668
Reviewed-by: Jack Franklin <[email protected]>
Commit-Queue: Tim van der Lippe <[email protected]>
babot pushed a commit to binaryage/dirac that referenced this issue Feb 20, 2020
This fixes an issue where property assignments to static methods (please
don't ask me why we have them) would erroneously omit the static method
from the declaration file.

This is fixed in the latest version of TypeScript, most likely as part
of microsoft/TypeScript#36270

DISABLE_THIRD_PARTY_CHECK=Updating TypeScript

Bug: 1011811
Change-Id: I2744ef44c040fdf001f6b0be9e64a164510e0fea
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2064668
Reviewed-by: Jack Franklin <[email protected]>
Commit-Queue: Tim van der Lippe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files Domain: JavaScript The issue relates to JavaScript specifically Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants