Skip to content

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

Merged
legobeat merged 15 commits intoMetaMask:developfrom
legobeat:deps-obj-multiplex
May 27, 2024
Merged

fix(deps): obj-multiplex@^1.0.0 -> @metamask/object-multiplex@^2.0.0#24533
legobeat merged 15 commits intoMetaMask:developfrom
legobeat:deps-obj-multiplex

Conversation

@legobeat
Copy link
Copy Markdown
Contributor

@legobeat legobeat commented May 14, 2024 •
edited
Loading

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
Copy link
Copy Markdown
Contributor

github-actions bot commented May 14, 2024

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 force-pushed the deps-obj-multiplex branch from c6e8090 to 621e855 Compare May 15, 2024 00:05
@legobeat legobeat added dependencies Pull requests that update a dependency file team-application-security Application security team 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
kumavis
kumavis reviewed May 21, 2024
View reviewed changes
kumavis
kumavis previously approved these changes May 21, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@kumavis kumavis left a comment

Choose a reason for hiding this comment

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

yay tests

@legobeat legobeat dismissed stale reviews from kumavis and rekmarks via bf3cef1 May 21, 2024 06:57
@legobeat legobeat force-pushed the deps-obj-multiplex branch from c8aa0f3 to bf3cef1 Compare May 21, 2024 06:57
@legobeat legobeat requested review from kumavis and rekmarks May 21, 2024 06:58
@legobeat
Copy link
Copy Markdown
Contributor Author

legobeat commented May 21, 2024

Had to resolve yarn.lock conflict after rebase due to merge of #24664.

No further changes.

@legobeat legobeat force-pushed the deps-obj-multiplex branch from bf3cef1 to 5d40c2f Compare May 21, 2024 07:48
@legobeat legobeat mentioned this pull request May 21, 2024
Merged
18 tasks
legobeat added 13 commits May 26, 2024 02:13
@legobeat
deps: obj-multiplex@^1.0.0 -> @metamask/object-multiplex@^2.0.0
af07c7f
@legobeat
chore: update lavamoat browserify policies
1171373
@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
@legobeat
chore: add implementors comments
7b4c75d
kumavis
kumavis previously approved these changes May 27, 2024
View reviewed changes
rekmarks
rekmarks reviewed 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
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 release-12.0.0 Issue or pull request that will be included in release 12.0.0 team-application-security Application security team

Projects

PR review queue
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@legobeat @kumavis @pedronfigueiredo @rekmarks @benjisclowder