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

json-rpc-engine: Wildcard exports -> named exports #4462

Merged
merged 2 commits into from
Jul 3, 2024

Conversation

mcmire
Copy link
Contributor

@mcmire mcmire commented Jun 25, 2024

Explanation

Using wildcard exports makes it difficult to understand what a package exports and makes it possible for new exports to get added accidentally.

This commit updates the @metamask/json-rpc-engine package to use named exports instead of wildcard exports, and adds a test to verify non-type exports following other packages.

References

This relates to the following ticket on eslint-config: MetaMask/eslint-config#331

I've created this PR as an example for Devin, an AI tool we are testing at Consensys. The idea is that Devin can use this PR to perform similar work on other packages in this repo.

Changelog

There should be no functional changes. All exports should be the same as they were before.

To verify this, search through packages/json-rpc-engine for exports (lines beginning with export) and cross-reference this with the list in index.ts.

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

Using wildcard exports makes it difficult to understand what a package
exports and makes it possible for new exports to get added accidentally.

This commit updates the `@metamask/json-rpc-engine` package to use named
exports instead of wildcard exports, and adds a test to verify non-type
exports following other packages.
@mcmire mcmire marked this pull request as ready for review June 25, 2024 22:01
@mcmire mcmire requested a review from a team as a code owner June 25, 2024 22:01
Copy link
Contributor

@MajorLift MajorLift left a comment

Choose a reason for hiding this comment

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

LGTM!

export * from './mergeMiddleware';
export type {
AsyncJsonRpcEngineNextCallback,
AsyncJsonrpcMiddleware,
Copy link
Contributor

Choose a reason for hiding this comment

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

optional: Maybe we should fix this here.

Suggested change
AsyncJsonrpcMiddleware,
AsyncJsonRpcMiddleware,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We really should, but I can do that in another PR.

@mcmire mcmire merged commit 7903e05 into main Jul 3, 2024
116 checks passed
@mcmire mcmire deleted the use-named-exports-for-json-rpc-engine branch July 3, 2024 21:20
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