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

Fails to generate typescript declarations from JS when extending EventEmitter #42405

Closed
vasco-santos opened this issue Jan 19, 2021 · 4 comments
Assignees
Labels
Fixed A PR has been merged for this issue Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros

Comments

@vasco-santos
Copy link

vasco-santos commented Jan 19, 2021

Bug Report

🔎 Search Terms

  • EventEmitter
  • TS9006
  • Declaration emit for this file requires using private name
  • An explicit type annotation may unblock declaration emit

🕗 Version & Regression Information

  • [email protected]
    • could not try this out with next versions as there are a few bugs already reported that will need to be fixed

This happened from 15th, more precisely after DefinitelyTyped/DefinitelyTyped#50266 being merged and released.

⏯ Playground Link

I believe I cannot add JS here 😞

You can look into libp2p before this PR being merged libp2p/js-libp2p#846.

Commit hash to use: d19401aa4cafcf2237af90b677c743b0ff0f871e. Just need npm i && npm run build

💻 Code

const { EventEmitter } = require('events')

class Manager extends EventEmitter {
  constructor () {
    super()
  }
}

module.exports = Manager

🙁 Actual behavior

Cannot generate typescript declaration file with errors like:

src/connection-manager/latency-monitor.js:2:1 - error TS9006: Declaration emit for this file requires using private name 'EventEmitter' from module '"events"'. An explicit type annotation may unblock declaration emit.

2  'use strict'

🙂 Expected behavior

Does not fail to build and generate the declaration files.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jan 21, 2021
@RyanCavanaugh
Copy link
Member

@weswigham seems WAI-ish, but is there anything more clever that could be done?

@weswigham
Copy link
Member

Considering the TS equivalent,

import { EventEmitter } from 'events'

class Manager extends EventEmitter {
  constructor () {
    super()
  }
}

export = Manager

works fine, and that JS should be bound pretty similarly to the above TS now, I'm thinking it's probably a bug somewhere in JS declaration binding.

@ExE-Boss
Copy link
Contributor

ExE-Boss commented Jan 22, 2021

What happens if:

import internal = require('events');
namespace EventEmitter {
	// Should just be `export { EventEmitter }`, but that doesn't work in TypeScript 3.4
	export { internal as EventEmitter };
}

in @types/node/events.d.ts is changed into:

namespace EventEmitter {
	// Note that this only works from TypeScript 3.5 onwards:
	export { EventEmitter };
}

Also, I can’t reproduce this on TypeScript 4.1.

@weswigham
Copy link
Member

Yeah, after looking at it, @ExE-Boss is right - this was fixed in 4.1. Check out the repro:

// @allowJs: true
// @checkJs: true
// @outDir: ./out
// @declaration: true
// @filename: node_modules/@types/node/index.d.ts
declare module "events" {
    import internal = require('events');
    class EventEmitter {
        member(): void;
    }
    namespace EventEmitter {
        // Should just be `export { EventEmitter }`, but that doesn't work in TypeScript 3.4
        export { internal as EventEmitter };
    }
    export = EventEmitter;
}

// @filename: file.js
/// <reference types="node" />

const { EventEmitter } = require('events')

class Manager extends EventEmitter {
  constructor () {
    super()
  }
}

module.exports = Manager

Workbench Repro

So, good news OP! It already works in 4.1 and above!

@typescript-bot typescript-bot added the Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros label Jan 22, 2021
@weswigham weswigham added Fixed A PR has been merged for this issue and removed Needs Investigation This issue needs a team member to investigate its status. labels Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed A PR has been merged for this issue Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros
Projects
None yet
Development

No branches or pull requests

5 participants