doc: migration best practices

README.md

@@ -9,7 +9,8 @@ This is a living repository — nothing is set in stone! If you're member of Met
 - [Engineering Principles](./docs/engineering-principles.md)
 - [Guide to Pull Requests](./docs/pull-requests.md)
 - [JavaScript Guidelines](./docs/javascript.md)
+- [Migrations Best Practices](./docs/migrations.md)
 - [Secure Development Lifecycle Policy](./docs/sdlc.md)
-- [Secure Coding Guidelines](./docs/docs/secure-coding-guidelines.md)
+- [Secure Coding Guidelines](./docs/secure-coding-guidelines.md)
 - [TypeScript Guidelines](./docs/typescript.md)
 - [Unit Testing Guidelines](./docs/unit-testing.md)

docs/mirgrations.md

@@ -0,0 +1,33 @@
+# State Migration Best Practices
+
+These best practices should apply to any team that contributes to an application that leverages local state.
+
+## Best Practices
+- When to Add a Migration: You should add a migration whenever you make a change to the shape of your state that is not backwards-compatible. This includes adding or removing properties, changing the type of a property, or moving properties around within the state tree (e.g. breaking controllers changes, etc...).
+- Detecting Errors in a Migration: The best way to detect errors in a migration is through thorough testing. Write tests that take various shapes of old state and ensure that they are correctly transformed into the new state shape. Also, use TypeScript or another type system to help catch type errors.
+- Handling Migration Errors: If a migration fails, you should have a strategy in place to handle the error. This could be as simple as logging the error and continuing with the default state, or it could involve more complex error recovery logic. In any case, it's important to ensure that your app can still function in some way even if a migration fails.
+
+- Example:
+```
+  if (
+    phishingControllerState?.hotlistLastFetched &&
+    phishingControllerState?.stalelistLastFetched
+  ) {
+    // This will make the list be fetched again when the user updates the app
+    state.engine.backgroundState.PhishingController.hotlistLastFetched = 0;
+    state.engine.backgroundState.PhishingController.stalelistLastFetched = 0;
+  } else {
+    captureException(
+      new Error(
+        `Migration 26: Invalid PhishingControllerState hotlist and stale list fetched: '${JSON.stringify(
+          state.engine.backgroundState.PhishingController,
+        )}'`,
+      ),
+    );
+  }
+
+  return state;
+```
+
+- Keeping Migrations Idempotent: Make sure your migrations are idempotent, meaning they can be run multiple times without changing the result beyond the initial application. This makes migrations safer and easier to test.
+- Removing Old Migrations: Over time, you may accumulate many migrations. At some point, it may be safe to remove old migrations, especially if you know that all users have migrated to a newer version of the state. Be cautious with this, as removing a migration could break the ability to upgrade for any users who are still on old versions of the state.