Skip to content

Commit

Permalink
Improve SnapController constructor typing (#2023)
Browse files Browse the repository at this point in the history
Improve SnapController constructor typing to improve compatibility with
mobile. Most of the constructor arguments have sane defaults provided by
the SnapController itself and are therefore allowed to be undefined. The
feature flags are also allowed to be true, false and undefined.

These issues weren't caught until now because the extension integration
is not typed.
  • Loading branch information
FrederikBolding authored Dec 8, 2023
1 parent f56ebf8 commit 57d8a53
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 13 deletions.
4 changes: 2 additions & 2 deletions packages/snaps-controllers/coverage.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"branches": 90.07,
"branches": 90.09,
"functions": 96.35,
"lines": 97.3,
"lines": 97.31,
"statements": 96.99
}
24 changes: 13 additions & 11 deletions packages/snaps-controllers/src/snaps/SnapController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -531,32 +531,32 @@ type SnapControllerMessenger = RestrictedControllerMessenger<
>;

type FeatureFlags = {
requireAllowlist?: true;
allowLocalSnaps?: true;
requireAllowlist?: boolean;
allowLocalSnaps?: boolean;
};

type SnapControllerArgs = {
/**
* A teardown function that allows the host to clean up its instrumentation
* for a running snap.
*/
closeAllConnections: CloseAllConnectionsFunction;
closeAllConnections?: CloseAllConnectionsFunction;

/**
* A list of permissions that are allowed to be dynamic, meaning they can be revoked from the snap whenever.
*/
dynamicPermissions: string[];
dynamicPermissions?: string[];

/**
* The names of endowment permissions whose values are the names of JavaScript
* APIs that will be added to the snap execution environment at runtime.
*/
environmentEndowmentPermissions: string[];
environmentEndowmentPermissions?: string[];

/**
* Excluded permissions with its associated error message used to forbid certain permssions.
*/
excludedPermissions: Record<string, string>;
excludedPermissions?: Record<string, string>;

/**
* The function that will be used by the controller fo make network requests.
Expand Down Expand Up @@ -666,7 +666,7 @@ export class SnapController extends BaseController<
SnapControllerState,
SnapControllerMessenger
> {
#closeAllConnections: CloseAllConnectionsFunction;
#closeAllConnections?: CloseAllConnectionsFunction;

#dynamicPermissions: string[];

Expand Down Expand Up @@ -1265,7 +1265,7 @@ export class SnapController extends BaseController<
runtime.pendingOutboundRequests = 0;
try {
if (this.isRunning(snapId)) {
this.#closeAllConnections(snapId);
this.#closeAllConnections?.(snapId);
await this.#terminateSnap(snapId);
}
} finally {
Expand Down Expand Up @@ -1444,9 +1444,11 @@ export class SnapController extends BaseController<
*/
async clearState() {
const snapIds = Object.keys(this.state.snaps);
snapIds.forEach((snapId) => {
this.#closeAllConnections(snapId);
});
if (this.#closeAllConnections) {
snapIds.forEach((snapId) => {
this.#closeAllConnections?.(snapId);
});
}

await this.messagingSystem.call('ExecutionService:terminateAllSnaps');
snapIds.forEach((snapId) => this.#revokeAllSnapPermissions(snapId));
Expand Down

0 comments on commit 57d8a53

Please sign in to comment.