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

Pulls in @types/node #5065

Closed
laino opened this issue Jun 21, 2024 · 3 comments
Closed

Pulls in @types/node #5065

laino opened this issue Jun 21, 2024 · 3 comments
Labels
bug Something isn't working package:engine.io-parser This concerns the "engine.io-parser" package

Comments

@laino
Copy link

laino commented Jun 21, 2024

In build/esm/index.d.ts and build/cjs/index.d.ts the following line is emitted:

/// <reference types="node" />

This will cause any (in-)direct import to pull in node types, polluting the global namespace.

To illustrate why this is an issue:

import { Socket } from 'socket.io-client';

//  error TS2322: Type 'Timeout' is not assignable to type 'number'.
const timeout: number = setTimeout(() => {}, 0);

The issue was introduced with version 5.2.2 and this commit: socketio/engine.io-parser@0305b4a

The last good version is 5.2.1.

@darrachequesne darrachequesne transferred this issue from socketio/engine.io-parser Jul 9, 2024
@darrachequesne darrachequesne added bug Something isn't working package:engine.io-parser This concerns the "engine.io-parser" package labels Jul 9, 2024
@darrachequesne
Copy link
Member

For history, socketio/engine.io-parser@0305b4a was introduced in order to provide a fix for #5064.

That being said, it seems TypeScript v5.5 does not include the /// <reference types="node" /> part anymore: https://devblogs.microsoft.com/typescript/announcing-typescript-5-5/#simplified-reference-directive-declaration-emit

Let me check if this doesn't break other configurations...

darrachequesne added a commit that referenced this issue Jul 11, 2024
The previous commit [1] tried to work around the fact that the
TransformStream object is not exposed in the global scope in the
`@types/node` package, even though it is since Node.js `v18.0.0`.

Unfortunately, it created two new issues:

- using an older `@types/node` version (before v16) would fail with:

> error TS2307: Cannot find module 'node:stream/web' or its corresponding type declarations.

Related: #5064 (comment)

- browser-only environments would somehow include the node types,
leading to conflicts like the return value of the setTimeout() method

Related:

- #5064 (comment)
- #5065

[1]: socketio/engine.io-parser@0305b4a
@darrachequesne
Copy link
Member

This should be fixed by f9cb983, included in engine.io-parser@5.2.3.

@laino could you please check?

@laino
Copy link
Author

laino commented Jul 11, 2024

It's fixed. Thanks!

@laino laino closed this as completed Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working package:engine.io-parser This concerns the "engine.io-parser" package
Projects
None yet
Development

No branches or pull requests

2 participants