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

@types's definition doesn't match its own type #35566

Open
falsandtru opened this issue Dec 7, 2019 · 11 comments
Open

@types's definition doesn't match its own type #35566

falsandtru opened this issue Dec 7, 2019 · 11 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@falsandtru
Copy link
Contributor

falsandtru commented Dec 7, 2019

DefinitelyTyped/DefinitelyTyped#40731 broke the following global declaration.

cc @rbuckton

TypeScript Version: 3.7.x-dev.20191207

Search Terms:

Code

https://github.com/falsandtru/spica/blob/v0.0.289/global.test.d.ts and @types/power-assert@1.5.1

import assert from 'power-assert';

type Assert = typeof assert;

declare global {
  const assert: Assert;
}

Expected behavior:
pass
Actual behavior:
TypeScript error: global.test.d.ts(6,9): Error TS2403: Subsequent variable declarations must have the same type. Variable 'assert' must be of type 'typeof assert', but here has type 'typeof assert'.
Playground Link:

Related Issues: #32808

@falsandtru falsandtru changed the title @types's definition doesn't match its own types @types's definition doesn't match its own type Dec 7, 2019
@rbuckton
Copy link
Member

rbuckton commented Dec 7, 2019

I see a couple things that could be problems that I didn't notice in the DT pull request:

  • Both power-assert/index.d.ts and power-assert/t3.7/index.d.ts have export as namespace assert;
  • The power-assert/ts3.7/index.d.ts imports power-assert/index.d.ts explicitly, so both of the export as namespace assert declarations exist and could possibly collide.

Also, you are declaring assert using typeof, which only gets the type-side of the declaration and not necessarily the namespace aspects of the declaration. You might try using the --allowUmdGlobalAccess flag, which would allow you to access the assert global namespace rather than redeclaring it via a global augmentation, but I'm not sure if you'll run into an issue due to the above collision.

We may need to change the DT definitions so that you aren't including both the root index.d.ts and ts3.7/index.d.ts.

@rbuckton
Copy link
Member

rbuckton commented Dec 7, 2019

I just checked this and the issue is due to the duplicate definitions of export as namespace in the root and ts3.7 versions.

@falsandtru
Copy link
Contributor Author

The power-assert/ts3.7/index.d.ts imports power-assert/index.d.ts explicitly, so both of the export as namespace assert declarations exist and could possibly collide.

I think so too. Looks like the compiler can't distinguish them.

@rbuckton
Copy link
Member

rbuckton commented Dec 7, 2019

The issue is that the declarations collide. I tested the following locally, which seems to fix the issue:

  1. Create a _common.d.ts file and copy the everything from index.d.ts into it except for the export as namespace declaration.
  2. Replace the source of index.d.ts, with the following:
    import assert = require("./_common");
    export = assert;
    export as namespace assert;
  3. Replace the import declaration in ts3.7/index.d.ts with this:
    import module = require("../_common");

Then TS won't attempt to load both files and the error goes away.

@falsandtru: This requires a DT PR to address. Do you want to do that or should I?

@rbuckton
Copy link
Member

rbuckton commented Dec 7, 2019

Alternatively, just reproduce the contents of index.d.ts in ts3.7/index.d.ts (with the adjustments for asserts conditions) and not have the import at all.

@falsandtru
Copy link
Contributor Author

I feel the first workaround is too complex. I want to choose the second. I'll make a PR. However, I think this is a bug anyway.

@rbuckton
Copy link
Member

rbuckton commented Dec 7, 2019

If we choose to address this bug, it will likely be by reporting a duplicate identifier error on each of the export as namespace assert declarations.

@rbuckton
Copy link
Member

rbuckton commented Dec 7, 2019

@falsandtru: Once you've made a PR, can you post a link to it here so that I can more easily review it?

@falsandtru
Copy link
Contributor Author

Sure.

@falsandtru
Copy link
Contributor Author

DefinitelyTyped/DefinitelyTyped#40903 is available.

rbuckton pushed a commit to DefinitelyTyped/DefinitelyTyped that referenced this issue Dec 8, 2019
@RyanCavanaugh RyanCavanaugh added the External Relates to another program, environment, or user action which we cannot control. label Dec 9, 2019
@falsandtru
Copy link
Contributor Author

falsandtru commented Dec 9, 2019

This problem is caused by the program written on this repository. When you resolve the problem you have to fix code on this repository. So this issue is not external.

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript and removed External Relates to another program, environment, or user action which we cannot control. labels Dec 9, 2019
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants