Skip to content

Circular typedef references causes incorrect error #34809

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
sandersn opened this issue Oct 29, 2019 · 6 comments · Fixed by #36457
Closed

Circular typedef references causes incorrect error #34809

sandersn opened this issue Oct 29, 2019 · 6 comments · Fixed by #36457
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@sandersn
Copy link
Member

// @filename: test.js
const mod1 = require('./mod1')
/** @typedef {number} CompilerStatus */
/** @type {CompilerStatus[]} */
var compilerStatus;
mod1;
module.exports = class C {
}

// @filename: mod1.js
/** @type {import("./test")} */
var megaman;
module.exports = megaman

Expected: No error, or maybe a circular reference error somewhere.

Actual: Error on @typedef {number} CompilerStatus: "Duplicate identifier 'CompilerStatus'.".

Bonus: in test.js, if you export module.exports = {} you get a stack overflow from the language service. I couldn't get this from the compiler, but I didn't try too hard.

@sandersn sandersn self-assigned this Oct 29, 2019
@sandersn sandersn added this to the TypeScript 3.8.0 milestone Oct 29, 2019
@sandersn sandersn added the Bug A bug in TypeScript label Oct 29, 2019
@sandersn sandersn changed the title Circular typedef references causes extraneous error Circular typedef references causes incorrect error Oct 29, 2019
@sandersn
Copy link
Member Author

From webpack: MultiCompiler.js and MultiWatching.js

@sandersn
Copy link
Member Author

Doesn't repro anymore, but webpack still has the same failure. Looks like I need to fix the repro.

@sandersn
Copy link
Member Author

I haven't got a repro but I've narrowed it down to a symbol-merging bug~~~~~!

I knew we shouldn't have removed that assert in mergeSymbolTable.

@sandersn
Copy link
Member Author

sandersn commented Jan 25, 2020

Looks like the first failure was on https://github.com/microsoft/TypeScript/pull/34718/files on 1:41 PM October 24. The run from October 23, 4:24 PM, does not have the failure.

@sandersn
Copy link
Member Author

Looks like it was caused by #34712

@sandersn
Copy link
Member Author

Here's a standalone repro:

{
  "compilerOptions": {
    "allowJs": true,
    "checkJs": true,
    "noEmit": true
  }
}

and

// @Filename: MW.js
/** @typedef {import("./MC")} MC */

class MW {
  /**
   * @param {MC} compiler the compiler
   */
  constructor(compiler) {
    this.compiler = compiler;
  }
}

module.exports = MW;

// @Filename: MC.js
const MW = require("./MW");

/** @typedef {number} Cictema */

module.exports = class MC {
  watch() {
    return new MW(this);
  }
};

Gives the following compile error:

MC.js:3:23 - error TS2300: Duplicate identifier 'Cictema'.

3 /** @typedef {number} Cictema */
                        ~~~~~~~

  MC.js:3:5
    3 /** @typedef {number} Cictema */
          ~~~~~~~~~~~~~~~~~~~~~~~~~
    'Cictema' was also declared here.
  MC.js:3:5
    3 /** @typedef {number} Cictema */
          ~~~~~~~~~~~~~~~~~~~~~~~~~
    and here.


Found 1 error.

The error appears in the editor too. I'm not really sure what's different from the repro I posted originally.

@sandersn sandersn added the Fix Available A PR has been opened for this issue label Jan 27, 2020
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants