Skip to content

fix: remove duplicate response logging in the browser console#1418

Merged
kettanaito merged 4 commits intomswjs:mainfrom
snaka:deps-strict-event-emitter
Oct 1, 2022
Merged

fix: remove duplicate response logging in the browser console#1418
kettanaito merged 4 commits intomswjs:mainfrom
snaka:deps-strict-event-emitter

Conversation

@snaka
Copy link
Copy Markdown
Contributor

@snaka snaka commented Sep 27, 2022

@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci bot commented Sep 27, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit b594029:

Sandbox Source
MSW React Configuration

@kettanaito
Copy link
Copy Markdown
Member

Thanks for adding this, @snaka!
We need to also add an integration test for this, as our CI didn't catch this issue being merged.

@snaka
Copy link
Copy Markdown
Contributor Author

snaka commented Sep 28, 2022

@kettanaito

I have a problem with that.
I don't know how to write or run Integration test.
Especially when I run integration test locally, I get the following error and all tests fail.

image

I ran yarn build before the test.
This is the situation on my current main branch.

My working environment is Windows (WSL2).
Is that the cause of the problem?

The tests run by GitHub Actions seemed to be using a virtual macOS, so I was wondering about that.

@snaka
Copy link
Copy Markdown
Contributor Author

snaka commented Sep 28, 2022

I followed the message and ran test:integration with the --detectOpenHandles option.

It seems that I am missing some libraries in my environment.

      Missing libraries we didn't find packages for:
          libexpat.so.1
          libgcc_s.so.1

I will install these libraries and give it a try.

image

@snaka
Copy link
Copy Markdown
Contributor Author

snaka commented Sep 28, 2022

Sorry.
It's still going to take a while to get Playwright to work on my Windows 10!
I will keep trying, but if you have any other suggestions, I would appreciate your advice.

@kettanaito
Copy link
Copy Markdown
Member

Hey, @snaka. No worries. I know it can be challenging to run PW on other systems. I will checkout your branch and help you with the test when I have a minute.

@snaka
Copy link
Copy Markdown
Contributor Author

snaka commented Sep 30, 2022

Thanks @kettanaito . It is very helpful.

@kettanaito kettanaito changed the title fix: update "strict-event-emitter" to fix duplicate log fix: remove duplicate response logging in the browser console Oct 1, 2022
@kettanaito
Copy link
Copy Markdown
Member

I've added the integration test for this fix 👍 Also found some shared state issue in the existing unit test, fixed that as well (I believe that one has been failing the CI).

@kettanaito
Copy link
Copy Markdown
Member

I've found another issue that events.removeAllListeners() didn't actually remove the listeners. Fixed in https://github.com/open-draft/strict-event-emitter/releases/tag/v0.2.6.

@kettanaito kettanaito merged commit 78d155f into mswjs:main Oct 1, 2022
@kettanaito
Copy link
Copy Markdown
Member

Thank you for helping fix this, @snaka! Welcome to contributors.

@snaka snaka deleted the deps-strict-event-emitter branch October 2, 2022 05:35
@snaka
Copy link
Copy Markdown
Contributor Author

snaka commented Oct 2, 2022

Thank you @kettanaito for your support.
Glad to hear the problem was resolved quickly!

@kettanaito
Copy link
Copy Markdown
Member

Released: v0.47.4 🎉

This has been released in v0.47.4!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

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.

Duplicate log output and MaxListenersExceededWarning warning (regression in 0.47.1)

2 participants