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.

Sign up for GitHub

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

Jump to bottom

Remove patched DOM types #3533

Merged
merged 2 commits into from
Sep 2, 2024
Merged

Remove patched DOM types #3533

merged 2 commits into from
Sep 2, 2024

Conversation

eXhumer
Copy link
Contributor

@eXhumer eXhumer commented Sep 1, 2024 •
edited
Loading

This relates to...

Fixes #3524

PR #3531

Rationale

Several missing DOM types from @types/node were added as part of patch.d.ts. Event & EventTarget exported DOM types in global were overwritten with patch.d.ts.

Changes

  • Remove patched Event & EventTarget type and references to it from other declaration files.
  • Lock version for @types/node to ~18.17.19.

Features

N/A

Bug Fixes

  • Fixes typing bug related to DOM Event type.

Breaking Changes and Deprecations

N/A

Status

Notes

@types/node version was updated to reflect the minimum supported engine version in package.json.

@eXhumer
Copy link
Contributor Author

eXhumer commented Sep 1, 2024

While back-porting this to v6.x branch, an additional declaration file (types/filereader.d.ts) needs to be modified to remove the patched imports as well.

@eXhumer
fix: Removed patched DOM types
e6e69be 
* see #3524

Signed-off-by: eXhumer <[email protected]>
@eXhumer
better @types/node version lock
5aa313d 
* Add minimum types for node based on engines.node version in package.json
* Currently undici locks engine to node@>=18.17
* Get @types/node for 18.17.x specificially and lock to patch upgrades only for 18.17

Signed-off-by: eXhumer <[email protected]>
@eXhumer
Copy link
Contributor Author

eXhumer commented Sep 1, 2024

Removed DOMException additionally as it is no longer used in v7. It was used in types/filereader.d.ts, but the file reader related types were removed and patched DOMException is not needed for undici v7. On v6.x branch, DOMException must not be removed as it is used in types/filereader.d.ts.

@mcollina
Copy link
Member

mcollina commented Sep 2, 2024

I'm reopening #3531 because it seems it cannot backported 1-1.

mcollina
mcollina approved these changes Sep 2, 2024
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Uzlopak Uzlopak mentioned this pull request Sep 2, 2024
Merged
5 tasks
mertcanaltin
mertcanaltin approved these changes Sep 2, 2024
View reviewed changes
Copy link
Member

@mertcanaltin mertcanaltin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit ce1ef21 into nodejs:main Sep 2, 2024
32 checks passed
@eXhumer eXhumer deleted the remove-patched-dom-types branch September 2, 2024 15:14
@github-actions github-actions bot mentioned this pull request Dec 3, 2024
Merged
This was referenced Dec 16, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mertcanaltin mertcanaltin mertcanaltin approved these changes

@mcollina mcollina mcollina approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unable to access base Event properties from CloseEvent, ErrorEvent & MessageEvent for WebSocket
3 participants
@eXhumer @mcollina @mertcanaltin