Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

In JS, assertions imported via commonjs have bogus error #38379

Closed
sandersn opened this issue May 6, 2020 · 5 comments · Fixed by #39770
Closed

In JS, assertions imported via commonjs have bogus error #38379

sandersn opened this issue May 6, 2020 · 5 comments · Fixed by #39770
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@sandersn
Copy link
Member

sandersn commented May 6, 2020

Code

// @filename: declares.d.ts
// this is the declaration for 'assert' from @types/node
export function art(value: any, message?: string | Error): asserts value;

// @filename: test.js
const { art } = require('./declares')
let x = 1
art(x)

Expected behavior:
No error.

Actual behavior:
Error: "Assertions require every name in the call target to be declared with an explicit type annotation."

There is no error when using ES imports:

// @filename: test2.js
import { art } from './declares'
let x = 1
art(x)
@sandersn
Copy link
Member Author

sandersn commented May 6, 2020

Repros since 3.7, when we introduced asserts.

@DanielRosenwasser
Copy link
Member

My hot take is that this is a bogus error almost everywhere you could get it...

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label May 8, 2020
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.0 milestone May 8, 2020
@Raynos
Copy link

Raynos commented Jun 15, 2020

I ran into this today when updating @types/node and using require('assert') from node core.

I fixed this with

/** @type {import('assert')} */
const assert = require('assert')

Is it possible for the type definition of require in TypeScript itself to return function require(module: T): import<T> ?

@sandersn
Copy link
Member Author

sandersn commented Jul 8, 2020

The problem is that symbols imported via require aren't actually aliases, so they don't get resolved in getTypeOfDottedName.

@sandersn
Copy link
Member Author

sandersn commented Jul 9, 2020

I prototyped a solution at the branch alias-for-require. It fixes this example, but is woefully incomplete everywhere else. It still needs:

  1. More complete handling of object literal syntax, which has way more options than the import syntax.
  2. Handling of un-destructured require.
  3. Rewrite of d.ts emit code.
  4. Replacement and removal of all the special-case fake-alias code elsewhere.
  5. Anything else I don't know about right now.

@weswigham you might be interested in this.

# for free to join this conversation on GitHub. Already have an account? # 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.

5 participants