Skip to content

Comments

fix(web-worker): ensure removeEventListener is bound to worker#7631

Merged
AriPerkkio merged 1 commit intovitest-dev:mainfrom
joelgallant:remove-event-listener-bind
Mar 11, 2025
Merged

fix(web-worker): ensure removeEventListener is bound to worker#7631
AriPerkkio merged 1 commit intovitest-dev:mainfrom
joelgallant:remove-event-listener-bind

Conversation

@joelgallant
Copy link
Contributor

@joelgallant joelgallant commented Mar 7, 2025 •
edited
Loading

Description

In jsdom + vitest web-workers, I get issues when calling removeEventListener from self.

'removeEventListener' called on an object that is not a valid instance of EventTarget

I tracked this down to the inserted context that web-worker inserts. This PR fixes it.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
  • Ideally, include a test that fails without this PR but passes with it.
  • Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

  • Run the tests with pnpm test:ci.

Screenshot From 2025-03-07 11-40-31

^ Test that fails before my patch

Documentation

  • If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

  • Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

@netlify
Copy link

netlify bot commented Mar 7, 2025

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 8033e5a
🔍 Latest deploy log https://app.netlify.com/sites/vitest-dev/deploys/67cb3d87efbdd70008c4101d
😎 Deploy Preview https://deploy-preview-7631--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@joelgallant joelgallant force-pushed the remove-event-listener-bind branch from 352991e to ee59971 Compare March 7, 2025 19:08
AriPerkkio
AriPerkkio approved these changes Mar 11, 2025
View reviewed changes
Copy link
Member

@AriPerkkio AriPerkkio left a comment

Choose a reason for hiding this comment

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

Thanks, good catch! 💯

@AriPerkkio AriPerkkio changed the title Ensure that removeEventListener is bound to the correct EventTarget fix(web-worker): ensure removeEventListener is bound to EventTarget Mar 11, 2025
@AriPerkkio AriPerkkio changed the title fix(web-worker): ensure removeEventListener is bound to EventTarget fix(web-worker): ensure removeEventListener is bound to worker Mar 11, 2025
@AriPerkkio AriPerkkio merged commit ff42bcb into vitest-dev:main Mar 11, 2025
10 of 13 checks passed
@tvanier tvanier mentioned this pull request Mar 17, 2025
Merged
@renovate renovate bot mentioned this pull request Mar 30, 2025
Merged
1 task
@renovate renovate bot mentioned this pull request May 5, 2025
Closed
1 task
This was referenced Jun 19, 2025
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hi-ogawa hi-ogawa hi-ogawa approved these changes

@AriPerkkio AriPerkkio AriPerkkio 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.

3 participants

@joelgallant @hi-ogawa @AriPerkkio