BREAKING: Remove :snapRemoved event (#2076)

FrederikBolding · web-flow · commit cfa78153d83f · 2024-01-08T12:03:27.000+01:00
After merging #2073, I realized that we might as well remove `:snapRemoved` too. It is more confusing than `:snapUninstalled` and all current use-cases can be covered with the previously mentioned event.
diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json
@@ -2,5 +2,5 @@
   "branches": 91.01,
   "functions": 96.35,
   "lines": 97.68,
-  "statements": 97.36
+  "statements": 97.35
 }
diff --git a/packages/snaps-controllers/src/cronjob/CronjobController.test.ts b/packages/snaps-controllers/src/cronjob/CronjobController.test.ts
@@ -322,7 +322,7 @@ describe('CronjobController', () => {
     cronjobController.destroy();
   });
 
-  it('handles SnapRemoved event', () => {
+  it('handles SnapUninstalled event', () => {
     const rootMessenger = getRootCronjobControllerMessenger();
     const controllerMessenger =
       getRestrictedCronjobControllerMessenger(rootMessenger);
@@ -345,7 +345,7 @@ describe('CronjobController', () => {
       MOCK_ORIGIN,
     );
 
-    rootMessenger.publish('SnapController:snapRemoved', snapInfo);
+    rootMessenger.publish('SnapController:snapUninstalled', snapInfo);
 
     jest.advanceTimersByTime(inMilliseconds(1, Duration.Minute));
 
diff --git a/packages/snaps-controllers/src/cronjob/CronjobController.ts b/packages/snaps-controllers/src/cronjob/CronjobController.ts
@@ -19,7 +19,7 @@ import type {
   SnapDisabled,
   SnapEnabled,
   SnapInstalled,
-  SnapRemoved,
+  SnapUninstalled,
   SnapUpdated,
 } from '..';
 import { getRunnableSnaps, SnapEndowments } from '..';
@@ -33,7 +33,7 @@ export type CronjobControllerActions =
 
 export type CronjobControllerEvents =
   | SnapInstalled
-  | SnapRemoved
+  | SnapUninstalled
   | SnapUpdated
   | SnapEnabled
   | SnapDisabled;
@@ -119,7 +119,7 @@ export class CronjobController extends BaseController<
     );
 
     this.messagingSystem.subscribe(
-      'SnapController:snapRemoved',
+      'SnapController:snapUninstalled',
       this._handleSnapUnregisterEvent,
     );
 
@@ -331,7 +331,7 @@ export class CronjobController extends BaseController<
     );
 
     this.messagingSystem.unsubscribe(
-      'SnapController:snapRemoved',
+      'SnapController:snapUninstalled',
       this._handleSnapUnregisterEvent,
     );
 
diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.ts b/packages/snaps-controllers/src/snaps/SnapController.test.ts
@@ -933,10 +933,6 @@ describe('SnapController', () => {
     );
 
     expect(controller.get(MOCK_SNAP_ID)).toBeUndefined();
-    expect(messenger.publish).toHaveBeenCalledWith(
-      'SnapController:snapRemoved',
-      getTruncatedSnap(),
-    );
 
     expect(messenger.publish).not.toHaveBeenCalledWith(
       'SnapController:snapUninstalled',
@@ -963,13 +959,6 @@ describe('SnapController', () => {
         throw new Error('foo');
       });
 
-    const eventSubscriptionPromise = new Promise<void>((resolve) => {
-      messenger.subscribe('SnapController:snapRemoved', (truncatedSnap) => {
-        expect(truncatedSnap).toStrictEqual(getTruncatedSnap());
-        resolve();
-      });
-    });
-
     await expect(
       snapController.installSnaps(MOCK_ORIGIN, {
         [MOCK_SNAP_ID]: {},
@@ -991,13 +980,6 @@ describe('SnapController', () => {
       }),
     );
 
-    await eventSubscriptionPromise;
-
-    expect(messenger.publish).toHaveBeenCalledWith(
-      'SnapController:snapRemoved',
-      getTruncatedSnap(),
-    );
-
     expect(messenger.publish).not.toHaveBeenCalledWith(
       'SnapController:snapUninstalled',
       getTruncatedSnap(),
@@ -1523,11 +1505,6 @@ describe('SnapController', () => {
 
     expect(snapController.state.snaps[snap.id]).toBeUndefined();
 
-    expect(messenger.publish).toHaveBeenCalledWith(
-      'SnapController:snapRemoved',
-      getTruncatedSnap(),
-    );
-
     expect(messenger.publish).toHaveBeenCalledWith(
       'SnapController:snapUninstalled',
       getTruncatedSnap(),
diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts
@@ -412,15 +412,6 @@ export type SnapUninstalled = {
   payload: [snap: TruncatedSnap];
 };
 
-/**
- * Emitted when a snap is removed from state, this may happen even
- * if a snap has not fully completed installation.
- */
-export type SnapRemoved = {
-  type: `${typeof controllerName}:snapRemoved`;
-  payload: [snap: TruncatedSnap];
-};
-
 /**
  * Emitted when an installed snap has been unblocked.
  */
@@ -475,7 +466,6 @@ export type SnapControllerEvents =
   | SnapBlocked
   | SnapInstalled
   | SnapUninstalled
-  | SnapRemoved
   | SnapStateChange
   | SnapUnblocked
   | SnapUpdated
@@ -1493,8 +1483,6 @@ export class SnapController extends BaseController<
           delete state.snapStates[snapId];
         });
 
-        this.messagingSystem.publish(`SnapController:snapRemoved`, truncated);
-
         // If the snap has been fully installed before, also emit snapUninstalled.
         if (snap.status !== SnapStatus.Installing) {
           this.messagingSystem.publish(
diff --git a/packages/snaps-controllers/src/test-utils/controller.ts b/packages/snaps-controllers/src/test-utils/controller.ts
@@ -333,7 +333,6 @@ export const getSnapControllerMessenger = (
       'SnapController:snapUninstalled',
       'SnapController:snapUnblocked',
       'SnapController:snapUpdated',
-      'SnapController:snapRemoved',
       'SnapController:stateChange',
       'SnapController:snapRolledback',
     ],
@@ -498,7 +497,7 @@ export const getRestrictedCronjobControllerMessenger = (
     allowedEvents: [
       'SnapController:snapInstalled',
       'SnapController:snapUpdated',
-      'SnapController:snapRemoved',
+      'SnapController:snapUninstalled',
       'SnapController:snapEnabled',
       'SnapController:snapDisabled',
     ],

Original file line number	Diff line number	Diff line change
`@@ -2,5 +2,5 @@`
`2`	`2`	`"branches": 91.01,`
`3`	`3`	`"functions": 96.35,`
`4`	`4`	`"lines": 97.68,`
`5`		`- "statements": 97.36`
	`5`	`+ "statements": 97.35`
`6`	`6`	`}`