Skip to content

Type reference to require import resolves to static side #34802

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 · 2 comments · Fixed by #34804
Closed

Type reference to require import resolves to static side #34802

sandersn opened this issue Oct 29, 2019 · 2 comments · Fixed by #34804
Assignees
Labels
Bug A bug in TypeScript Domain: JavaScript The issue relates to JavaScript specifically Fix Available A PR has been opened for this issue

Comments

@sandersn
Copy link
Member

// @filename: ex.d.ts
export class C {
  start: number
  end: number
}
// @filename: test.js
const C = require('./ex').C
const C = require('./ex').C;
/** @type {C} c */
var c = new C()

Expected: No error, and completions for c.start and c.end

Actual: Error, "Property 'prototype' is missing in type 'C' but required in type 'typeof C'."

@sandersn sandersn self-assigned this Oct 29, 2019
@sandersn sandersn added Bug A bug in TypeScript Domain: JavaScript The issue relates to JavaScript specifically labels Oct 29, 2019
@sandersn sandersn added this to the TypeScript 3.7.2 milestone Oct 29, 2019
@sandersn
Copy link
Member Author

sandersn commented Oct 29, 2019

The new syntactic check for require calls in getTypeFromJSDocValueReference looks for exactly = require(STRING) but doesn't look for the value equivalent of a "import type with entity name". It's easy to fix, but this makes me wonder whether the syntactic approach is the right one.

sandersn added a commit that referenced this issue Oct 29, 2019
For example, `require("x").c`. This is the value equivalent of
`import("x").a.b.c`, but the syntax tree is not as nicely designed for
this purpose.

Fixes #34802
@sandersn
Copy link
Member Author

On the other hand, the check I added is equivalent to import("x").a.b, which only has two forms, typeof being the other, so it's likely that this check is good enough.

@sandersn sandersn added the Fix Available A PR has been opened for this issue label Oct 29, 2019
sandersn added a commit that referenced this issue Oct 29, 2019
* resolve require with entity name postfix

For example, `require("x").c`. This is the value equivalent of
`import("x").a.b.c`, but the syntax tree is not as nicely designed for
this purpose.

Fixes #34802

* Add bug number to test

* Add optional chain test
sandersn added a commit that referenced this issue Oct 29, 2019
* resolve require with entity name postfix

For example, `require("x").c`. This is the value equivalent of
`import("x").a.b.c`, but the syntax tree is not as nicely designed for
this purpose.

Fixes #34802

* Add bug number to test

* Add optional chain test
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: 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.

1 participant