pubnub / javascript

PubNub JavaScript SDK docs https://www.pubnub.com/docs/sdks/javascript
Other
553 stars 401 forks source link

Reference tripple-slash directives in typings pollute global environment #409

Closed yo1dog closed 1 week ago

yo1dog commented 1 month ago

All /// <reference .../> should be removed as they pollute the global environment.

For reference: https://github.com/microsoft/TypeScript/issues/33111

Superagent and many other packages have made the same mistake and also removed: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/57641

Mostly harmless in this case, but could cause conflicting Node.js typings to be referenced.

parfeon commented 3 weeks ago

Thank you for reaching out to us.

There are no lib dependencies in compilerOptions for tsc, but it looks like to add those because it spotted some imports. With processing type definition aggregation script, those references will be ignored.

yo1dog commented 3 weeks ago

Sounds good.

But to be clear, if a /// <reference .../> tripple-slash directive exists anywhere in the import tree it will still pollute the global scope. So the aggregated definitions file must not import/reference any file which includes /// <reference .../> either.

parfeon commented 3 weeks ago

@yo1dog this issue is addressed in v8.2.10