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

fix(deps): obj-multiplex@^1.0.0 -> @metamask/object-multiplex@^2.0.0 #24533

Merged
merged 15 commits into from
May 27, 2024
Merged

fix(deps): obj-multiplex@^1.0.0 -> @metamask/object-multiplex@^2.0.0 #24533

merged 15 commits into from
May 27, 2024

Conversation

legobeat
Copy link
Contributor

@legobeat legobeat commented May 14, 2024 •
edited

Description

obj-multiplex has been renamed to @metamask/obj-multiplex.

  • Upgrade from obj-multiplex@1.0.0 to obj-multiplex@2.0.0
  • Adjust handling for attempts at writing to destroyed streams
    • Could be indicative of issue on receiving side or resource leaking, but here it is ignored/swallowed like it already was.

readable-stream.pipeline is equivalent to legacy pump - this moves to the former for the surrounding usage involving ObjMultiplex instances.

Open in GitHub Codespaces

Related issues

Broken out from

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've completed the PR template to the best of my ability
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@github-actions GitHub Actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@legobeat legobeat changed the title deps: obj-multiplex@^1.0.0 -> @metamask/object-multiplex@^2.0.0 fix(deps): obj-multiplex@^1.0.0 -> @metamask/object-multiplex@^2.0.0 May 14, 2024
@legobeat legobeat added dependencies Pull requests that update a dependency file team-security labels May 15, 2024
@legobeat legobeat force-pushed the deps-obj-multiplex branch 3 times, most recently from 74f9b74 to b6e2cc7 Compare May 15, 2024 03:08
@legobeat legobeat marked this pull request as ready for review May 15, 2024 03:08
@legobeat legobeat requested review from a team as code owners May 15, 2024 03:08
@legobeat legobeat force-pushed the deps-obj-multiplex branch 17 times, most recently from 686ce19 to a0e6c66 Compare May 17, 2024 04:24
@legobeat legobeat requested a review from kumavis May 17, 2024 04:40
@legobeat
fix: check if multiplex stream is writable before attempting write
4355ee1 
Note: The existing _writableState.ended can be removed when we are
certain legacy streams won't be used here.

https://npm-diff.app/readable-stream@2.3.8...readable-stream@3.6.2#lib/_stream_writable.js-L266
@legobeat
chore: refactor out isStreamWritable function
d80a8ca
@legobeat
WIP: Add test for isStreamWritable
8cf5fab
@legobeat
devDeps: add readable-stream-2, readable-stream-3 as alias for readab…
4795818 
…le-stream by version
@legobeat
chore: Test isStreamWritable using different readable-stream implemen…
ca29d7e 
…tations
@legobeat
chore: add test-cases for isStreamWritable
fea58a7
@legobeat
fix: isStreamWritable always returns bool
c0b7db9
@legobeat
chore: fixup test
01e1196
@legobeat
chore: add test-case for isStreamWritable
f797f26
@legobeat
chore: add test-cases for isStreamWritable
4cb095e
kumavis
kumavis previously approved these changes May 27, 2024
View reviewed changes
@legobeat @rekmarks
chore: Update inline comment
55c1086 
Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
rekmarks
rekmarks previously approved these changes May 27, 2024
View reviewed changes
@legobeat legobeat requested review from kumavis and a team May 27, 2024 02:48
@legobeat legobeat requested a review from rekmarks May 27, 2024 03:34
@legobeat legobeat force-pushed the deps-obj-multiplex branch 3 times, most recently from cacc69a to 704c404 Compare May 27, 2024 05:16
@legobeat legobeat merged commit 72d4b52 into MetaMask:develop May 27, 2024
71 of 72 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators May 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kumavis kumavis kumavis approved these changes

@pedronfigueiredo pedronfigueiredo pedronfigueiredo approved these changes

@jiexi jiexi Awaiting requested review from jiexi

@adonesky1 adonesky1 Awaiting requested review from adonesky1

@naugtur naugtur Awaiting requested review from naugtur

@rekmarks rekmarks Awaiting requested review from rekmarks

Assignees
No one assigned
Labels
dependencies Pull requests that update a dependency file team-security
Projects
PR review queue
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@legobeat @kumavis @pedronfigueiredo @rekmarks