Skip to content

eth-json-rpc-provider migration - Integration into packages/ #1738

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

Merged
MajorLift merged 9 commits into from
Oct 13, 2023
Merged

eth-json-rpc-provider migration - Integration into packages/ #1738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a0f04f8
Move `eth-json-rpc-provider` into `packages/` directory.
MajorLift Oct 3, 2023
58b7e5e
Install `eth-json-rpc-provider` as a dependency of the monorepo.
MajorLift Oct 3, 2023
fe985a2
Add tsconfig reference paths for `eth-json-rpc-provider` in root and …
MajorLift Oct 3, 2023
26dcd51
Bump `eth-json-rpc-provider` dep versions in root and downstream pack…
MajorLift Oct 3, 2023
7befb4e
Apply validator fixes for `eth-json-rpc-provider` `CHANGELOG.md`
MajorLift Oct 3, 2023
b3338ce
Apply constraints fixes to `eth-json-rpc-provider` package.json file
MajorLift Oct 3, 2023
f02b824
Fix lockfile
MajorLift Oct 4, 2023
4c8fea7
Export `SafeEventEmitterProvider` as class instead of type
MajorLift Oct 12, 2023
a32e68a
Fix import statements for `SafeEventEmitterProvider`
MajorLift Oct 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
"@metamask/eslint-config-jest": "^12.1.0",
"@metamask/eslint-config-nodejs": "^12.1.0",
"@metamask/eslint-config-typescript": "^12.1.0",
"@metamask/eth-json-rpc-provider": "^2.1.0",
"@metamask/eth-json-rpc-provider": "^2.2.0",
"@metamask/json-rpc-engine": "^7.1.1",
"@metamask/utils": "^8.1.0",
"@types/node": "^16.18.54",
Expand Down
16 changes: 8 additions & 8 deletions ...ckages/eth-json-rpc-provider/CHANGELOG.md → packages/eth-json-rpc-provider/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ Release `v2.0.0` is identical to `v1.0.1` aside from Node.js version requirement

## [1.0.0]
### Added
- Initial release, including `providerFromEngine` and `providerFromMiddleware`

[Unreleased]: https://github.com/MetaMask/eth-json-rpc-provider/compare/v2.2.0...HEAD
[2.2.0]: https://github.com/MetaMask/eth-json-rpc-provider/compare/v2.1.0...v2.2.0
[2.1.0]: https://github.com/MetaMask/eth-json-rpc-provider/compare/v2.0.0...v2.1.0
[2.0.0]: https://github.com/MetaMask/eth-json-rpc-provider/compare/v1.0.1...v2.0.0
[1.0.1]: https://github.com/MetaMask/eth-json-rpc-provider/compare/v1.0.0...v1.0.1
[1.0.0]: https://github.com/MetaMask/eth-json-rpc-provider/releases/tag/v1.0.0
- Initial release, including `providerFromEngine` and `providerFromMiddleware`.

[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/eth-json-rpc-provider@2.2.0...HEAD
[2.2.0]: https://github.com/MetaMask/core/compare/@metamask/eth-json-rpc-provider@2.1.0...@metamask/eth-json-rpc-provider@2.2.0
[2.1.0]: https://github.com/MetaMask/core/compare/@metamask/eth-json-rpc-provider@2.0.0...@metamask/eth-json-rpc-provider@2.1.0
[2.0.0]: https://github.com/MetaMask/core/compare/@metamask/eth-json-rpc-provider@1.0.1...@metamask/eth-json-rpc-provider@2.0.0
[1.0.1]: https://github.com/MetaMask/core/compare/@metamask/eth-json-rpc-provider@1.0.0...@metamask/eth-json-rpc-provider@1.0.1
[1.0.0]: https://github.com/MetaMask/core/releases/tag/@metamask/eth-json-rpc-provider@1.0.0
0 ...ed-packages/eth-json-rpc-provider/LICENSE → packages/eth-json-rpc-provider/LICENSE
View file
Open in desktop
File renamed without changes.
0 ...-packages/eth-json-rpc-provider/README.md → packages/eth-json-rpc-provider/README.md
View file
Open in desktop
File renamed without changes.
0 ...ages/eth-json-rpc-provider/jest.config.js → ...ages/eth-json-rpc-provider/jest.config.js
View file
Open in desktop
File renamed without changes.
25 changes: 18 additions & 7 deletions ...ckages/eth-json-rpc-provider/package.json → packages/eth-json-rpc-provider/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,47 +2,58 @@
"name": "@metamask/eth-json-rpc-provider",
"version": "2.2.0",
"description": "Create an Ethereum provider using a JSON-RPC engine or middleware",
"keywords": [
"MetaMask",
"Ethereum"
],
"homepage": "https://github.com/MetaMask/core/tree/main/packages/eth-json-rpc-provider#readme",
"bugs": {
"url": "https://github.com/MetaMask/core/issues"
},
"repository": {
"type": "git",
"url": "https://github.com/MetaMask/eth-json-rpc-provider.git"
"url": "https://github.com/MetaMask/core.git"
},
"license": "ISC",
"main": "dist/index.js",
"types": "dist/index.d.ts",
"main": "./dist/index.js",
"types": "./dist/index.d.ts",
"files": [
"dist/"
],
"scripts": {
"build": "tsc --project tsconfig.build.json",
"build:clean": "rimraf dist && yarn build",
"build:docs": "typedoc",
"changelog:validate": "../../scripts/validate-changelog.sh @metamask/eth-json-rpc-provider",
"lint": "yarn lint:eslint && yarn lint:misc --check && yarn lint:dependencies",
"lint:dependencies": "depcheck",
"lint:eslint": "eslint . --cache --ext js,ts",
"lint:fix": "yarn lint:eslint --fix && yarn lint:misc --write && yarn lint:dependencies",
"lint:misc": "prettier '**/*.json' '**/*.md' '!CHANGELOG.md' '**/*.yml' '!.yarnrc.yml' --ignore-path .gitignore --no-error-on-unmatched-pattern",
"prepack": "./scripts/prepack.sh",
"test": "jest && jest-it-up",
"publish:preview": "yarn npm publish --tag preview",
"test": "jest",
"test:clean": "jest --clearCache",
"test:watch": "jest --watch"
},
"dependencies": {
"@metamask/json-rpc-engine": "^7.1.0",
"@metamask/json-rpc-engine": "^7.1.1",
"@metamask/safe-event-emitter": "^3.0.0",
"@metamask/utils": "^8.1.0"
},
"devDependencies": {
"@metamask/auto-changelog": "^3.1.0",
"@types/jest": "^27.4.1",
"depcheck": "^1.4.3",
"deepmerge": "^4.2.2",
"depcheck": "^1.4.3",
"jest": "^27.5.1",
"jest-it-up": "^2.0.2",
"ts-jest": "^27.1.4",
"typedoc": "^0.24.8"
},
"packageManager": "yarn@3.3.0",
"engines": {
"node": "^16.20 || ^18.16 || >=20"
"node": ">=16.0.0"
},
"publishConfig": {
"access": "public",
Expand Down
1 change: 1 addition & 0 deletions ...s/eth-json-rpc-provider/src/index.test.ts → ...s/eth-json-rpc-provider/src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ describe('Package exports', () => {
it('has expected exports', () => {
expect(Object.keys(allExports)).toMatchInlineSnapshot(`
Array [
"SafeEventEmitterProvider",
Copy link
Contributor

@mcmire mcmire Oct 12, 2023

Choose a reason for hiding this comment

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

What's the reason for this?

Copy link
Contributor Author

@MajorLift MajorLift Oct 12, 2023

Choose a reason for hiding this comment

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

This one follows from the export statement change for SafeEventEmitterProvider https://github.com/MetaMask/core/pull/1738/files#r1357548245. The test is checking what non-types are being exported from index.ts.

"providerFromEngine",
"providerFromMiddleware",
]
Expand Down
2 changes: 1 addition & 1 deletion ...ckages/eth-json-rpc-provider/src/index.ts → packages/eth-json-rpc-provider/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export * from './provider-from-engine';
export * from './provider-from-middleware';
export type { SafeEventEmitterProvider } from './safe-event-emitter-provider';
Copy link
Contributor

@mcmire mcmire Oct 12, 2023

Choose a reason for hiding this comment

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

This changes what is being exported (the whole class instead of just the type). What's the reason for this change?

Copy link
Contributor Author

@MajorLift MajorLift Oct 12, 2023

Choose a reason for hiding this comment

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

See: first line of network-controller/tests/fake-provider.

Copy link
Contributor

@mcmire mcmire Oct 13, 2023

Choose a reason for hiding this comment

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

Okay, no problem. I guess this wouldn't be a breaking change, so it's okay.

export { SafeEventEmitterProvider } from './safe-event-emitter-provider';
0 ...provider/src/provider-from-engine.test.ts → ...provider/src/provider-from-engine.test.ts
View file
Open in desktop
File renamed without changes.
0 ...-rpc-provider/src/provider-from-engine.ts → ...-rpc-provider/src/provider-from-engine.ts
View file
Open in desktop
File renamed without changes.
0 ...ider/src/provider-from-middleware.test.ts → ...ider/src/provider-from-middleware.test.ts
View file
Open in desktop
File renamed without changes.
0 ...-provider/src/provider-from-middleware.ts → ...-provider/src/provider-from-middleware.ts
View file
Open in desktop
File renamed without changes.
0 ...r/src/safe-event-emitter-provider.test.ts → ...r/src/safe-event-emitter-provider.test.ts
View file
Open in desktop
File renamed without changes.
0 ...ovider/src/safe-event-emitter-provider.ts → ...ovider/src/safe-event-emitter-provider.ts
View file
Open in desktop
File renamed without changes.
0 ...eth-json-rpc-provider/tsconfig.build.json → ...eth-json-rpc-provider/tsconfig.build.json
View file
Open in desktop
File renamed without changes.
0 ...kages/eth-json-rpc-provider/tsconfig.json → packages/eth-json-rpc-provider/tsconfig.json
View file
Open in desktop
File renamed without changes.
0 ...ckages/eth-json-rpc-provider/typedoc.json → packages/eth-json-rpc-provider/typedoc.json
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion packages/network-controller/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"@metamask/controller-utils": "^5.0.2",
"@metamask/eth-json-rpc-infura": "^9.0.0",
"@metamask/eth-json-rpc-middleware": "^12.0.0",
"@metamask/eth-json-rpc-provider": "^2.1.0",
"@metamask/eth-json-rpc-provider": "^2.2.0",
Copy link
Contributor

@mcmire mcmire Oct 12, 2023

Choose a reason for hiding this comment

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

Hmm. Given that we are affecting other packages in this PR, should we bump everything to ^2.2.0 first? I think it'd be better to keep this PR just focused to eth-json-rpc-provider if we can.

Copy link
Contributor Author

@MajorLift MajorLift Oct 12, 2023

Choose a reason for hiding this comment

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

This is necessary because of the reference path shadowing thing we discussed. Once eth-json-rpc-provider is moved into packages/, downstream packages can no longer use previous versions by pointing to node_modules/, because all imports from @metamask/eth-json-rpc-provider will always point to the path in packages/, and therefore to the latest version. This seems to happen even if the migrated package isn't added to the reference paths in the downstream pkgs' typescript config files.

Copy link
Contributor

@mcmire mcmire Oct 12, 2023

Choose a reason for hiding this comment

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

That makes sense, but isn't this happening because network-controller is currently using version 2.1.0? If we bumped that first then the latest version of the package being used and current version of the package as it exists after being migrated would be the same.

Copy link
Contributor Author

@MajorLift MajorLift Oct 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We could bump eth-json-rpc-provider to 2.2.0 everywhere first in a separate PR, but that PR wouldn't include the changes being made to network-controller/ here (the last two commits).

Because fixing the ../dist/.. import statements requires fixing that export statement in eth-json-rpc-provider, we'd still end up with changes being made to both packages simultaneously before the migration can be merged.

Copy link
Contributor Author

@MajorLift MajorLift Oct 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I guess we could change the SafeEventEmitterProvider export statement in the original repo and release 2.2.1 first, but that would mean redoing the migration process from the start because of the new git history.

"@metamask/eth-query": "^3.0.1",
"@metamask/json-rpc-engine": "^7.1.1",
"@metamask/rpc-errors": "^6.1.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/network-controller/tests/fake-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SafeEventEmitterProvider } from '@metamask/eth-json-rpc-provider/dist/safe-event-emitter-provider';
import { SafeEventEmitterProvider } from '@metamask/eth-json-rpc-provider';
Copy link
Contributor Author

@MajorLift MajorLift Oct 12, 2023

Choose a reason for hiding this comment

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

See line 85:

export class FakeProvider extends SafeEventEmitterProvider {

Importing SafeEventEmitterProvider as a type breaks that line, because it's being used as a class.

This is why the previous import statement pointed directly to node_modules/.../dist/... instead of using the SafeEventEmitterProvider type exported by the package.

That import statement can no longer be used (it won't point to node_modules with eth-json-rpc-provider in packages/) which is why the export from eth-json-rpc-provider needs to be fixed from type to class.

Copy link
Contributor

@mcmire mcmire Oct 13, 2023

Choose a reason for hiding this comment

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

Ah, I see. Thank you for explaining, somehow that was not connecting.

import { JsonRpcEngine } from '@metamask/json-rpc-engine';
import type { JsonRpcRequest, JsonRpcResponse } from '@metamask/utils';
import { inspect, isDeepStrictEqual } from 'util';
Expand Down
3 changes: 2 additions & 1 deletion packages/network-controller/tsconfig.build.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
},
"references": [
{ "path": "../base-controller/tsconfig.build.json" },
{ "path": "../controller-utils/tsconfig.build.json" }
{ "path": "../controller-utils/tsconfig.build.json" },
{ "path": "../eth-json-rpc-provider/tsconfig.build.json" }
],
"include": ["../../types", "./src"]
}
3 changes: 3 additions & 0 deletions packages/network-controller/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
},
{
"path": "../controller-utils"
},
{
"path": "../eth-json-rpc-provider"
}
],
"include": ["../../types", "../../tests", "./src", "./tests"]
Expand Down
2 changes: 1 addition & 1 deletion tests/fake-block-tracker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SafeEventEmitterProvider } from '@metamask/eth-json-rpc-provider/dist/safe-event-emitter-provider';
import { SafeEventEmitterProvider } from '@metamask/eth-json-rpc-provider';
import { JsonRpcEngine } from '@metamask/json-rpc-engine';
import { PollingBlockTracker } from 'eth-block-tracker';

Expand Down
3 changes: 3 additions & 0 deletions tsconfig.build.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
{
"path": "./packages/ens-controller/tsconfig.build.json"
},
{
"path": "./packages/eth-json-rpc-provider/tsconfig.build.json"
},
{
"path": "./packages/gas-fee-controller/tsconfig.build.json"
},
Expand Down
3 changes: 3 additions & 0 deletions tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
{
"path": "./packages/ens-controller"
},
{
"path": "./packages/eth-json-rpc-provider"
},
{
"path": "./packages/gas-fee-controller"
},
Expand Down
Loading