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

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

Open
yo1dog opened this issue Oct 14, 2024 · 3 comments
Open
Assignees
Labels
priority: low This PR should be reviewed after all high and medium PRs. status: in progress This issue is being worked on. type: chore This PR contains changes that are not covered by other types (stylistic, dependency updates, etc).

Comments

@yo1dog
Copy link

yo1dog commented Oct 14, 2024

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

For reference: microsoft/TypeScript#33111

Superagent and many other packages have made the same mistake and also removed: DefinitelyTyped/DefinitelyTyped#57641

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

@parfeon
Copy link
Contributor

parfeon commented Oct 29, 2024

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.

@parfeon parfeon self-assigned this Oct 29, 2024
@parfeon parfeon added status: in progress This issue is being worked on. priority: low This PR should be reviewed after all high and medium PRs. type: chore This PR contains changes that are not covered by other types (stylistic, dependency updates, etc). labels Oct 29, 2024
@yo1dog
Copy link
Author

yo1dog commented Oct 29, 2024

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 added a commit that referenced this issue Oct 30, 2024
Fix definition of type which represent message actions received from history and list of users which added action of specific type and value to the message.

Closes #407

refactor(types): remove indexed signature for publish

Remove redundant indexed signature from publish message parameters type definition.

Closes #413

refactor(types): add serializable objects to `Payload` type

Extend `Payload` type definition with objects which can be serialized by `JSON.stringify` using `toJSON()` methods.

Closes #412

refactor(types): aggregate generated types definitions

Aggregate multiple types definitions into single type definition type with proper type names and namespaces.

Closes #405 #409 #410

refactor(types): add missing Subscribe Event Engine types

Add Subscribe Event Engine and Event Listener types to the bundled types definition file.

Closes #377
parfeon added a commit that referenced this issue Oct 31, 2024
fix(types): fix `Actions` type definition

Fix definition of type which represent message actions received from history and list of users which added action of specific type and value to the message.

Closes #407

refactor(types): remove indexed signature for publish

Remove redundant indexed signature from publish message parameters type definition.

Closes #413

refactor(types): add serializable objects to `Payload` type

Extend `Payload` type definition with objects which can be serialized by `JSON.stringify` using `toJSON()` methods.

Closes #412

refactor(types): aggregate generated types definitions

Aggregate multiple types definitions into single type definition type with proper type names and namespaces.

Closes #405 #409 #410

refactor(types): add missing Subscribe Event Engine types

Add Subscribe Event Engine and Event Listener types to the bundled types definition file.

Closes #377
@parfeon
Copy link
Contributor

parfeon commented Oct 31, 2024

@yo1dog this issue is addressed in v8.2.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: low This PR should be reviewed after all high and medium PRs. status: in progress This issue is being worked on. type: chore This PR contains changes that are not covered by other types (stylistic, dependency updates, etc).
Projects
None yet
Development

No branches or pull requests

2 participants