Skip to content

Commit

Permalink
Fix permission-controller lint violations (#4537)
Browse files Browse the repository at this point in the history 
There are some `permission-controller` tests which attempt to push an
async middleware function onto an engine. Although this works in
practice, `@typescript-eslint` balks at this, because the type for
`JsonRpcEngine.push` says it takes a `JsonRpcMiddleware`, and a
`JsonRpcMiddleware` has a return type of `void`, not `Promise<void>`. We
could change that return type to `void | Promise<void>` but that would
cause other changes we may not want to make. So this commit merely
ignores the lint violations. This should be safe because `JsonRpcEngine`
doesn't await middleware anyway.

Interestingly, the lint violations caused by this discrepancy only
appear locally and not on CI. I am not sure why this is.
  • Loading branch information
@mcmire
mcmire authored Jul 18, 2024
1 parent b7fa70e commit ed7a9db
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 18 deletions.
14 changes: 10 additions & 4 deletions packages/permission-controller/src/rpc-methods/getPermissions.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@ describe('getPermissions RPC method', () => {

const engine = new JsonRpcEngine();
engine.push(
async (
(
req: JsonRpcRequest<[]>,
res: PendingJsonRpcResponse<PermissionConstraint[]>,
next,
end,
) => {
await implementation(req, res, next, end, {
// We intentionally do not await this promise; JsonRpcEngine won't await
// middleware anyway.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
implementation(req, res, next, end, {
getPermissionsForOrigin: mockGetPermissionsForOrigin,
});
},
Expand All @@ -44,13 +47,16 @@ describe('getPermissions RPC method', () => {

const engine = new JsonRpcEngine();
engine.push(
async (
(
req: JsonRpcRequest<[]>,
res: PendingJsonRpcResponse<PermissionConstraint[]>,
next,
end,
) => {
await implementation(req, res, next, end, {
// We intentionally do not await this promise; JsonRpcEngine won't await
// middleware anyway.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
implementation(req, res, next, end, {
getPermissionsForOrigin: mockGetPermissionsForOrigin,
});
},
Expand Down
14 changes: 10 additions & 4 deletions packages/permission-controller/src/rpc-methods/requestPermissions.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@ describe('requestPermissions RPC method', () => {

const engine = new JsonRpcEngine();
engine.push<[RequestedPermissions], PermissionConstraint[]>(
async (req, res, next, end) => {
await implementation(req, res, next, end, {
(req, res, next, end) => {
// We intentionally do not await this promise; JsonRpcEngine won't await
// middleware anyway.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
implementation(req, res, next, end, {
requestPermissionsForOrigin: mockRequestPermissionsForOrigin,
});
},
Expand Down Expand Up @@ -102,8 +105,11 @@ describe('requestPermissions RPC method', () => {

const engine = new JsonRpcEngine();
engine.push<[RequestedPermissions], PermissionConstraint[]>(
async (req, res, next, end) => {
await implementation(req, res, next, end, {
(req, res, next, end) => {
// We intentionally do not await this promise; JsonRpcEngine won't await
// middleware anyway.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
implementation(req, res, next, end, {
requestPermissionsForOrigin: mockRequestPermissionsForOrigin,
});
},
Expand Down
35 changes: 25 additions & 10 deletions packages/permission-controller/src/rpc-methods/revokePermissions.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,11 @@ describe('revokePermissions RPC method', () => {
const mockRevokePermissionsForOrigin = jest.fn();

const engine = new JsonRpcEngine();
engine.push<RevokePermissionArgs, null>(async (req, res, next, end) => {
await implementation(req, res, next, end, {
engine.push<RevokePermissionArgs, null>((req, res, next, end) => {
// We intentionally do not await this promise; JsonRpcEngine won't await
// middleware anyway.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
implementation(req, res, next, end, {
revokePermissionsForOrigin: mockRevokePermissionsForOrigin,
});
});
Expand Down Expand Up @@ -57,8 +60,11 @@ describe('revokePermissions RPC method', () => {
const mockRevokePermissionsForOrigin = jest.fn();

const engine = new JsonRpcEngine();
engine.push<RevokePermissionArgs, null>(async (req, res, next, end) => {
await implementation(req, res, next, end, {
engine.push<RevokePermissionArgs, null>((req, res, next, end) => {
// We intentionally do not await this promise; JsonRpcEngine won't await
// middleware anyway.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
implementation(req, res, next, end, {
revokePermissionsForOrigin: mockRevokePermissionsForOrigin,
});
});
Expand Down Expand Up @@ -91,8 +97,11 @@ describe('revokePermissions RPC method', () => {
const mockRevokePermissionsForOrigin = jest.fn();

const engine = new JsonRpcEngine();
engine.push<RevokePermissionArgs, null>(async (req, res, next, end) => {
await implementation(req, res, next, end, {
engine.push<RevokePermissionArgs, null>((req, res, next, end) => {
// We intentionally do not await this promise; JsonRpcEngine won't await
// middleware anyway.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
implementation(req, res, next, end, {
revokePermissionsForOrigin: mockRevokePermissionsForOrigin,
});
});
Expand Down Expand Up @@ -125,8 +134,11 @@ describe('revokePermissions RPC method', () => {
const mockRevokePermissionsForOrigin = jest.fn();

const engine = new JsonRpcEngine();
engine.push<RevokePermissionArgs, null>(async (req, res, next, end) => {
await implementation(req, res, next, end, {
engine.push<RevokePermissionArgs, null>((req, res, next, end) => {
// We intentionally do not await this promise; JsonRpcEngine won't await
// middleware anyway.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
implementation(req, res, next, end, {
revokePermissionsForOrigin: mockRevokePermissionsForOrigin,
});
});
Expand Down Expand Up @@ -158,8 +170,11 @@ describe('revokePermissions RPC method', () => {
const mockRevokePermissionsForOrigin = jest.fn();

const engine = new JsonRpcEngine();
engine.push<RevokePermissionArgs, null>(async (req, res, next, end) => {
await implementation(req, res, next, end, {
engine.push<RevokePermissionArgs, null>((req, res, next, end) => {
// We intentionally do not await this promise; JsonRpcEngine won't await
// middleware anyway.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
implementation(req, res, next, end, {
revokePermissionsForOrigin: mockRevokePermissionsForOrigin,
});
});
Expand Down

0 comments on commit ed7a9db

Please sign in to comment.