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

TypeScript: 0.8.1 breaks re-exported on #78

Closed
SimenB opened this issue Feb 18, 2021 · 9 comments · Fixed by #93
Closed

TypeScript: 0.8.1 breaks re-exported on #78

SimenB opened this issue Feb 18, 2021 · 9 comments · Fixed by #93

Comments

@SimenB
Copy link

SimenB commented Feb 18, 2021

import Emittery = require('emittery');

type Events = {
  foobar: string;
}

export class MyThing {
  private readonly eventEmitter = new Emittery<Events>();

  on = this.eventEmitter.on.bind(this.eventEmitter);
}

Using 0.8.0 this works fine, but upgrading to 0.8.1 gives

src/index.ts:10:3 - error TS2527: The inferred type of 'on' references an inaccessible 'unique symbol' type. A type annotation is necessary.

10   on = this.eventEmitter.on.bind(this.eventEmitter);
     ~~

src/index.ts:10:3 - error TS4029: Public property 'on' of exported class has or is using name 'listenerAdded' from external module "/Users/simen/repos/emittery-stuff/node_modules/emittery/index" but cannot be named.

10   on = this.eventEmitter.on.bind(this.eventEmitter);
     ~~

src/index.ts:10:3 - error TS4029: Public property 'on' of exported class has or is using name 'listenerRemoved' from external module "/Users/simen/repos/emittery-stuff/node_modules/emittery/index" but cannot be named.

10   on = this.eventEmitter.on.bind(this.eventEmitter);
     ~~


Found 3 errors.

Comes from #73.

I've put together a small repo showing this: https://github.com/SimenB/emittery-type-error.

Note that this only happens if you produce declaration files from source

@sindresorhus
Copy link
Owner

// @airhorns @papb

@airhorns
Copy link
Contributor

@SimenB would you expect your re-exported listener to be able to listen to the internal events of the event emitter?

@SimenB
Copy link
Author

SimenB commented Feb 18, 2021

Not really, that's more of an implementation detail. So for my use case that's not needed 🙂

@airhorns
Copy link
Contributor

Ok, in that case I think you should just add an explicit type annotation like the TS compiler suggests! My bad for accidentally causing this breaking change, but I think in this case the compiler is right, and this just surfaced an error that was hidden by the incorrect bits in index.d.ts.

@airhorns
Copy link
Contributor

If @sindresorhus we could yank 0.8.1 and republish as 0.9, but I think this rebinding use case is pretty niche and 0.8.1 has been out for long enough that would cause a lot of chagrin on all the other adopters

@sindresorhus
Copy link
Owner

I don't see the point of yanking the current version. Maybe we should improve the docs though.

@beckend
Copy link
Contributor

beckend commented Feb 4, 2022

This is still an issue, where one wish to make a library that uses this one.
Simply export will fix this:

declare const listenerAdded: unique symbol;
declare const listenerRemoved: unique symbol;

export {
  listenerAdded,
  listenerRemoved
}

It's for the ability to generate declarations.

@airhorns
Copy link
Contributor

airhorns commented Feb 4, 2022

@beckend want to make a PR?

@beckend
Copy link
Contributor

beckend commented Feb 4, 2022

@airhorns did the minimal touch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants