benlesh/symbol-observable

Types don't use `unique symbol` type

pittst3r opened this issue · 3 comments

Consumers of this library may do something like this:

import $$observable from 'symbol-observable';

export class Foo {
  [$$observable]() {
    return this;
  }
}

If using TypeScript their output .d.ts will look like the following, because $$observable is not a unique symbol:

export declare class Foo {
}

However, if we use the unique symbol type the output .d.ts will be:

import $$observable from 'symbol-observable';
export declare class Foo {
   [$$observable](): this;
}

This allows improves interop between observable interfaces with regards to typing (rxjs and xstream in my case). There are unfortunately more issues between rxjs and xstream; namely the Subscribable interfaces.

Would you accept a PR amending the types to use the unique symbol type? Note that using unique symbol would require a TS upgrade to ~2.7.0 and an update to rxjs so as to not regress ReactiveX/rxjs#3697.

This apparently is also at odds with @types/node, so we can't really make this fix until it's either fixed or removed there.

Also of note: symbol-observable@3.0.0 is updated to publish types for Symbol.observable as being unique symbol as well.

I'm still seeing this issue with symbol-observable@3.0.0 and @types/node@14.14.35...

Error: node_modules/symbol-observable/index.d.ts:6:14 - error TS2717: Subsequent property declarations must have the same type.  Property 'observable' must be of type 'symbol', but here has type 'unique symbol'.

6     readonly observable: unique symbol;
               ~~~~~~~~~~

  node_modules/@types/node/globals.d.ts:146:14
    146     readonly observable: symbol;
                     ~~~~~~~~~~
    'observable' was also declared here.

Update: Installing an older 2.x version of this package as noted in apollographql/apollo-client#7337 fixed the issue, still unsure why the latest of both wouldn't work though. Lmk if you'd like me to create a separate issue.