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

fix: unambiguously detect web workers #1728

Merged
merged 4 commits into from
Oct 30, 2023

Conversation

f1234k
Copy link
Contributor

@f1234k f1234k commented Oct 29, 2023

Use the same method that browser-or-node NPM package uses to detect web workers. This has been tested against my use case for the issue described here, and it works as expected: the Bun runtime is not being (erroneously) identified as a web worker.

P.S: Apologies for cancelling the PR initially, but I didn't feel right about it, and I had to run some more tests against my project.

robertsLando
robertsLando previously approved these changes Oct 30, 2023
@robertsLando
Copy link
Member

@f1234k #1724 also edits this files so you may need to fix conflicts (seems there are also lint issues)

@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f6123f2) 81.03% compared to head (faf695d) 81.03%.

❗ Current head faf695d differs from pull request most recent head 8c26dc7. Consider uploading reports for the commit 8c26dc7 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1728   +/-   ##
=======================================
  Coverage   81.03%   81.03%           
=======================================
  Files          21       21           
  Lines        1366     1366           
  Branches      323      323           
=======================================
  Hits         1107     1107           
  Misses        182      182           
  Partials       77       77           
Files Coverage Δ
src/lib/is-browser.ts 66.66% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@f1234k
Copy link
Contributor Author

f1234k commented Oct 30, 2023

@robertsLando conflicts with #1724 have been resolved, so have the linting issues.

Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

Please remove the bun.lockdb and pacacke-lock.json changes from the PR

@f1234k
Copy link
Contributor Author

f1234k commented Oct 30, 2023

@robertsLando sorry for that. Should be fine now.

Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

No worries! LGTM

@robertsLando robertsLando enabled auto-merge (squash) October 30, 2023 13:13
@robertsLando robertsLando merged commit e44368c into mqttjs:main Oct 30, 2023
3 checks passed
@f1234k f1234k deleted the detect-web-workers branch October 30, 2023 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants