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

Remove unnecessary babel-runtime dependencies #1504

Merged
merged 2 commits into from
Jul 14, 2023

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Jul 14, 2023

Explanation

We included babel-runtime as a dependency to a few packages because we thought it was an undocumented dependency of eth-query, but we had gotten it mixed up with ethjs-query. Really we only need this as a dependency in the transaction controller.

The constraints have been updated, and all unnecessary references to babel-runtime have been removed.

Additionally, in the one package where it's still required, babel-runtime has been made into a peerDependency instead of a direct dependency. This better reflects the requirement: we don't need to use it directly, we just need it to be available in the environment this package is run in.

References

The missing babel-runtime dependency was discovered in #341

Changelog

@metamask/assets-controllers

  • Changed: Remove babel-runtime dependency

@metamask/controller-utils

  • Changed: Remove babel-runtime dependency

@metamask/network-controller

  • Changed: Remove babel-runtime dependency

@metamask/transaction-controller

  • BREAKING: Change babel-runtime from a dependency to a peerDependency

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@Gudahtt Gudahtt marked this pull request as ready for review July 14, 2023 03:38
@Gudahtt Gudahtt requested a review from a team as a code owner July 14, 2023 03:38
@legobeat
Copy link
Contributor

What do you think about additionally moving the remaining babel-runtime dependency in transaction-controller to peerDependencies (which ideally nonce-tracker should be speccing in the first place)? Seems more semantic.

@Gudahtt
Copy link
Member Author

Gudahtt commented Jul 14, 2023

Great idea! Done in 2320069

We included `babel-runtime` as a dependency to a few packages because
we thought it was an undocumented dependency of `eth-query`, but we had
gotten it mixed up with `ethjs-query`. Really we only need this as a
dependency in the transaction controller.

The constraints have been updated, and all unnecessary references to
`babel-runtime` have been removed.
@Gudahtt Gudahtt force-pushed the remove-unnecessary-babel-runtime-dependencies branch from 2320069 to 91820c3 Compare July 14, 2023 04:45
@Gudahtt Gudahtt merged commit ce615a2 into main Jul 14, 2023
91 checks passed
@Gudahtt Gudahtt deleted the remove-unnecessary-babel-runtime-dependencies branch July 14, 2023 05:01
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
* Remove unnecessary `babel-runtime` dependencies

We included `babel-runtime` as a dependency to a few packages because
we thought it was an undocumented dependency of `eth-query`, but we had
gotten it mixed up with `ethjs-query`. Really we only need this as a
dependency in the transaction controller.

The constraints have been updated, and all unnecessary references to
`babel-runtime` have been removed.

* Move `babel-runtime` to `peerDependencies`
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
* Remove unnecessary `babel-runtime` dependencies

We included `babel-runtime` as a dependency to a few packages because
we thought it was an undocumented dependency of `eth-query`, but we had
gotten it mixed up with `ethjs-query`. Really we only need this as a
dependency in the transaction controller.

The constraints have been updated, and all unnecessary references to
`babel-runtime` have been removed.

* Move `babel-runtime` to `peerDependencies`
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