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: clean 'close' event listeners on socket server after generating new proxy config. #5001

Merged
merged 3 commits into from
Jan 20, 2024

Conversation

cwj0417
Copy link
Contributor

@cwj0417 cwj0417 commented Sep 23, 2023

  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

For Bugs and Features; did you add new tests?

yes, test cases added.

Motivation / Use-Case

when pass a function to proxy config. every time firing handler function, a new proxyServer were set, and events on old proxyServer has not been cleared.

Breaking Changes

no

Additional Info

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 23, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@alexander-akait
Copy link
Member

Thank you

@jarias-lfx
Copy link

/easycla

@alexander-akait
Copy link
Member

Ignore failed CI, we will fix it soon

@cwj0417
Copy link
Contributor Author

cwj0417 commented Dec 21, 2023

Ignore failed CI, we will fix it soon

thank you for your reminder, please mention me when something need to be fixed.

@cwj0417
Copy link
Contributor Author

cwj0417 commented Dec 22, 2023

I removed optional chaining and nullish coalescing operator to make code compatible with lower node versions.
I'm sorry for not noticing before.

Copy link

codecov bot commented Dec 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (540c438) 90.38% compared to head (30aa7ad) 90.52%.
Report is 65 commits behind head on next.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #5001      +/-   ##
==========================================
+ Coverage   90.38%   90.52%   +0.13%     
==========================================
  Files          16       16              
  Lines        1706     1710       +4     
  Branches      649      652       +3     
==========================================
+ Hits         1542     1548       +6     
+ Misses        150      148       -2     
  Partials       14       14              

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

@alexander-akait
Copy link
Member

@snitin315 I want to merge it (you can do it too), will be it hard to rebase the next?

@snitin315
Copy link
Member

No rebase won't be hard. But we can directly merge it in the next branch too

@alexander-akait alexander-akait changed the base branch from master to next January 20, 2024 13:24
@alexander-akait alexander-akait merged commit 7a3eab9 into webpack:next Jan 20, 2024
60 of 67 checks passed
alexander-akait pushed a commit that referenced this pull request Jan 22, 2024
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.

4 participants