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

deps: upgrade deep-equal@1.1.0->2.1.0 #90

Closed
wants to merge 1 commit into from

Conversation

legobeat
Copy link
Contributor

@legobeat legobeat commented Dec 12, 2022

No description provided.

@legobeat legobeat marked this pull request as ready for review December 12, 2022 23:16
@legobeat legobeat requested a review from a team as a code owner December 12, 2022 23:16
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

This seems to add a lot more dependencies. Is that okay? (I assume maybe so in this case, as maybe they are small, but just want to check.)

@legobeat

This comment was marked as outdated.

@legobeat
Copy link
Contributor Author

legobeat commented Dec 13, 2022

To address thew question about size, when measured by yarn install --prod: This commit is an increase of 3284 b (27368 - 24084).

The main contributor here is object.assign clocking in at 1284b. It seems the maintainer is optimizing heavily for backwards-compatibility: inspect-js/node-deep-equal#100

This package is using babel to address that. I think you're right @mcmire that this one isn't worth it. Thank you for catching that!

Is object.assign et al worth filling in for still? Might be worth replacing deep-equal alltogether for something more appropriate.

Alternatively, 2.0.2 is the last version not depending on object.assign.

@legobeat legobeat closed this Dec 13, 2022
@legobeat
Copy link
Contributor Author

(then on the other hand, in context of metamask-extension, we already have a runtime dependency on the same version of object.assign 🤷 )

@legobeat
Copy link
Contributor Author

legobeat commented Apr 24, 2023

@mcmire I resurrected this in #94, with the ambition of getting out a non-breaking maintenance release before deprecating nodejs v12, which would also unlock bringing this in line with metamask-module-template.

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.

2 participants