feat(laboratory): scope environment variables to target

.changeset/rich-terms-knock.md

@@ -0,0 +1,20 @@
+---
+'hive': minor
+---
+
+Laboratory Environment Variables are now scoped to Target.
+
+Previously, we stored environment variables as a static key in your browser's local storage. This meant that any changes to the environment variables would affect all targets' Laboratory.
+
+Now when you use Laboratory, any changes to the environment variables will not affect the environment variables of other targets' Laboratory.
+
+## Migration Details (TL;DR: You Won't Notice Anything!)
+
+For an indefinite period of time we will support the following migration when you load Laboratory on any target. If this holds true:
+
+1. Your browser's localStorage has a key for the global environment variables;
+2. Your browser's localStorage does NOT have a key for scoped environment variables for the Target Laboratory being loaded;
+
+Then we will initialize the scoped environment variables for the Target Laboratory being loaded with the global ones.
+
+Laboratory will _never_ write to the global environment variables again, so this should give you a seamless migration to scoped environment variables for all your targets.

cypress.config.ts

@@ -23,7 +23,7 @@ export default defineConfig({
   video: isCI,
   screenshotOnRunFailure: isCI,
   defaultCommandTimeout: 15_000, // sometimes the app takes longer to load, especially in the CI
-  retries: 2,
+  retries: isCI ? 2 : 0,
   e2e: {
     setupNodeEvents(on) {
       on('task', {
@@ -33,6 +33,11 @@ export default defineConfig({
           const project = await org.createProject();
           const slug = `${org.organization.slug}/${project.project.slug}/${project.target.slug}`;
           return {
+            targets: {
+              production: project.targets.find(_ => _.name === 'production'),
+              staging: project.targets.find(_ => _.name === 'staging'),
+              development: project.targets.find(_ => _.name === 'development'),
+            },
             slug,
             refreshToken: owner.ownerRefreshToken,
             email: owner.ownerEmail,

cypress/e2e/laboratory-environment-variables.cy.ts

@@ -0,0 +1,84 @@
+const selectors = {
+  editorEnvironmentVariables: '[data-cy="preflight-editor-mini"]',
+  buttonGraphiQLPreflight: '[aria-label*="Preflight Script"]',
+  buttonModalCy: 'preflight-modal-button',
+  buttonToggleCy: 'toggle-preflight',
+  buttonHeaders: '[data-name="headers"]',
+  headersEditor: {
+    textArea: '.graphiql-editor-tool .graphiql-editor:last-child textarea',
+  },
+  graphiql: {
+    buttonExecute: '.graphiql-execute-button',
+  },
+
+  modal: {
+    buttonSubmitCy: 'preflight-modal-submit',
+  },
+};
+
+// todo: instead of copying this, import it from core utility lib.
+export const environmentVariablesStorageKey = {
+  // todo: optional target effectively gives this the possibility of being silently global
+  // which feels subtle and thus likely to introduce hard to trace defects. Should we abort instead?
+  scoped: (targetId?: string) =>
+    `hive/targetId:${targetId ?? '__null__'}/laboratory/environment-variables`,
+  global: 'hive:laboratory:environment',
+};
+
+const data = {
+  envars: { foo: '123' },
+  envarsJson: '{"foo":"123"}',
+};
+
+const as = <$Type>() => undefined as $Type;
+
+const ctx = {
+  // test export interfaces from testKit
+  targetDevelopment: as<{ id: string; slug: string; path: string }>(),
+  targetProduction: as<{ id: string; slug: string; path: string }>(),
+};
+
+before(() => {
+  cy.clearLocalStorage().then(async () => {
+    cy.task('seedTarget').then(({ refreshToken, targets }: any) => {
+      cy.setCookie('sRefreshToken', refreshToken);
+      ctx.targetDevelopment = targets.development;
+      ctx.targetProduction = targets.production;
+    });
+  });
+});
+
+const visitTargetDevelopment = () => cy.visit(`/${ctx.targetDevelopment.path}/laboratory`);
+const visitTargetProduction = () => cy.visit(`/${ctx.targetProduction.path}/laboratory`);
+const openPreflightTab = () => cy.get(selectors.buttonGraphiQLPreflight).click();
+
+describe('tab editor', () => {
+  it('if state empty, is null', () => {
+    visitTargetDevelopment();
+    openPreflightTab();
+    expect(
+      window.localStorage.getItem(
+        environmentVariablesStorageKey.scoped(ctx.targetDevelopment.slug),
+      ),
+    ).equals(null);
+    expect(window.localStorage.getItem(environmentVariablesStorageKey.global)).equals(null);
+    // cy.dataCy('preflight-editor-mini').should('have.text', '');
+  });
+
+  it('if state just global value, shows that', () => {
+    window.localStorage.setItem(environmentVariablesStorageKey.global, data.envarsJson);
+    visitTargetDevelopment();
+    openPreflightTab();
+    cy.contains(data.envarsJson);
+  });
+
+  it.only('if state just scoped value, shows that', () => {
+    window.localStorage.setItem(
+      environmentVariablesStorageKey.scoped(ctx.targetDevelopment.id),
+      data.envarsJson,
+    );
+    visitTargetDevelopment();
+    openPreflightTab();
+    cy.contains(data.envarsJson);
+  });
+});

integration-tests/testkit/seed.ts

@@ -210,9 +210,12 @@ export function initSeed() {
                 ownerToken,
               ).then(r => r.expectNoGraphQLErrors());
 
-              const targets = projectResult.createProject.ok!.createdTargets;
-              const target = targets[0];
               const project = projectResult.createProject.ok!.createdProject;
+              const targets = projectResult.createProject.ok!.createdTargets.map(_ => ({
+                ..._,
+                path: `/${organization.slug}/${project.slug}/${_.slug}`,
+              }));
+              const target = targets[0];
 
               return {
                 project,

packages/web/app/src/lib/hooks/use-local-storage-json.ts

@@ -1,9 +1,12 @@
 import { useCallback, useState } from 'react';
 import { z } from 'zod';
 import { Kit } from '../kit';
+import { readVersionedEntryLocalStorage, VersionedEntrySpec } from '../versioned-entry';
 
 export function useLocalStorageJson<$Schema extends z.ZodType>(...args: ArgsInput<$Schema>) {
   const [key, schema, manualDefaultValue] = args as any as Args<$Schema>;
+  const versionedEntry: VersionedEntrySpec = typeof key === 'string' ? [{ key }] : key;
+
   // The parameter types will force the user to give a manual default
   // if their given Zod schema does not have default.
   //
@@ -24,7 +27,7 @@ export function useLocalStorageJson<$Schema extends z.ZodType>(...args: ArgsInpu
     // because we manually pre-compute+return the default value, thus we don't
     // rely on Zod's behaviour. If that changes this should have `?? undefined`
     // added.
-    const storedValue = localStorage.getItem(key);
+    const storedValue = readVersionedEntryLocalStorage({ spec: versionedEntry });
 
     if (!storedValue) {
       return defaultValue;
@@ -49,27 +52,29 @@ export function useLocalStorageJson<$Schema extends z.ZodType>(...args: ArgsInpu
 
   const set = useCallback(
     (value: z.infer<$Schema>) => {
-      localStorage.setItem(key, JSON.stringify(value));
+      localStorage.setItem(versionedEntry[0].key, JSON.stringify(value));
       setValue(value);
     },
-    [key],
+    [versionedEntry.map(({ key }) => key).join('+')],
   );
 
   return [value, set] as const;
 }
 
 type ArgsInput<$Schema extends z.ZodType> =
   $Schema extends z.ZodDefault<z.ZodType>
-    ? [key: string, schema: ArgsInputGuardZodJsonSchema<$Schema>]
-    : [key: string, schema: ArgsInputGuardZodJsonSchema<$Schema>, defaultValue: z.infer<$Schema>];
+    ? [key: KeyInput, schema: ArgsInputGuardZodJsonSchema<$Schema>]
+    : [key: KeyInput, schema: ArgsInputGuardZodJsonSchema<$Schema>, defaultValue: z.infer<$Schema>];
 
 type ArgsInputGuardZodJsonSchema<$Schema extends z.ZodType> =
   z.infer<$Schema> extends Kit.Json.Value
     ? $Schema
     : 'Error: Your Zod schema is or contains a type that is not valid JSON.';
 
 type Args<$Schema extends z.ZodType> = [
-  key: string,
+  key: KeyInput,
   schema: $Schema,
   defaultValue?: z.infer<$Schema>,
 ];
+
+type KeyInput = string | VersionedEntrySpec;

packages/web/app/src/lib/hooks/use-local-storage.ts

@@ -1,17 +1,27 @@
-import { useCallback, useState } from 'react';
+import { useCallback, useEffect, useState } from 'react';
+import { readVersionedEntryLocalStorage, VersionedEntrySpec } from '../versioned-entry';
 
-export function useLocalStorage(key: string, defaultValue: string) {
-  const [value, setValue] = useState<string>(() => {
-    const value = localStorage.getItem(key);
+export function useLocalStorage(key: string | VersionedEntrySpec, defaultValue: string) {
+  const versionedEntry: VersionedEntrySpec = typeof key === 'string' ? [{ key }] : key;
+  const versionedEntrySerialized = versionedEntry.map(_ => _.key).join(',');
+
+  const getInitialValue = useCallback(() => {
+    const value = readVersionedEntryLocalStorage({ spec: versionedEntry });
     return value ?? defaultValue;
-  });
+  }, [versionedEntrySerialized, defaultValue]);
+
+  const [value, setValue] = useState(getInitialValue());
+
+  useEffect(() => {
+    setValue(getInitialValue());
+  }, [getInitialValue]);
 
   const set = useCallback(
     (value: string) => {
-      localStorage.setItem(key, value);
+      localStorage.setItem(versionedEntry[0].key, value);
       setValue(value);
     },
-    [setValue],
+    [getInitialValue],
   );
 
   return [value, set] as const;

packages/web/app/src/lib/preflight/graphiql-plugin.tsx

@@ -144,6 +144,14 @@ export const enum PreflightWorkerState {
   ready,
 }
 
+export const environmentVariablesStorageKey = {
+  // todo: optional target effectively gives this the possibility of being silently global
+  // which feels subtle and thus likely to introduce hard to trace defects. Should we abort instead?
+  scoped: (targetId?: string) =>
+    `hive/targetId:${targetId ?? '__null__'}/laboratory/environment-variables`,
+  global: 'hive:laboratory:environment',
+};
+
 export function usePreflight(args: {
   target: FragmentType<typeof PreflightScript_TargetFragment> | null;
 }) {
@@ -152,13 +160,16 @@ export function usePreflight(args: {
 
   const target = useFragment(PreflightScript_TargetFragment, args.target);
   const [isEnabled, setIsEnabled] = useLocalStorageJson(
-    // todo: ability to pass historical keys for seamless gradual migration to new key names.
+    // todo
     // 'hive:laboratory:isPreflightEnabled',
     'hive:laboratory:isPreflightScriptEnabled',
     z.boolean().default(false),
   );
   const [environmentVariables, setEnvironmentVariables] = useLocalStorage(
-    'hive:laboratory:environment',
+    [
+      { key: environmentVariablesStorageKey.scoped(target?.id) },
+      { key: environmentVariablesStorageKey.global },
+    ],
     '',
   );
   const latestEnvironmentVariablesRef = useRef(environmentVariables);

packages/web/app/src/lib/versioned-entry.spec.ts

@@ -0,0 +1,60 @@
+import {
+  createKeyValueStoreMemory,
+  KeyValueStoreDatabase,
+  PreviousEntriesPolicy,
+  readVersionedEntry,
+  VersionedEntrySpec,
+} from './versioned-entry';
+
+interface TestCase {
+  databaseBefore: KeyValueStoreDatabase;
+  databaseAfter: KeyValueStoreDatabase;
+  spec: VersionedEntrySpec;
+  value: string | null;
+  previousEntriesPolicy?: PreviousEntriesPolicy;
+}
+
+const a = 'a';
+const b = 'b';
+const c = 'c';
+
+// prettier-ignore
+test.for<TestCase>([
+	// Returns null if spec key is missing in db
+	{ spec: [{ key:a }],                   databaseBefore: {},      databaseAfter: {},      value: null },
+	{ spec: [{ key:a }],                   databaseBefore: {b},     databaseAfter: {b},     value: null },
+	// Returns value if spec key is present in db
+	{ spec: [{ key:a }],                   databaseBefore: {a},     databaseAfter: {a},     value: a },
+	{ spec: [{ key:a }],                   databaseBefore: {a,b},   databaseAfter: {a,b},   value: a },
+	{ spec: [{ key:a }, {key:b}],          databaseBefore: {a},     databaseAfter: {a},     value: a },
+	//
+	// With previousEntriesPolicy = ignore (default)
+	//
+	// Previous spec keys are NOT removed from db
+	{ spec: [{ key:a }, {key:b}],          databaseBefore: {a,b},   databaseAfter: {a,b},       value: a },
+	{ spec: [{ key:a }, {key:b}, {key:c}], databaseBefore: {a,b,c}, databaseAfter: {a,b,c},     value: a },
+	// Latest found spec key is returned
+	{ spec: [{ key:a }, {key:b}],          databaseBefore: {b},     databaseAfter: {a:b,b},     value: b },
+	{ spec: [{ key:a }, {key:b}, {key:c}], databaseBefore: {c},     databaseAfter: {a:c,c},     value: c },
+	{ spec: [{ key:a }, {key:b}, {key:c}], databaseBefore: {b,c},   databaseAfter: {a:b,b,c},   value: b },
+	//
+	// With previousEntriesPolicy = remove
+	//
+	// Previous spec keys are removed from db
+	{ spec: [{ key:a }, {key:b}],          databaseBefore: {a,b},   databaseAfter: {a},     value: a, previousEntriesPolicy: 'remove' },
+	{ spec: [{ key:a }, {key:b}, {key:c}], databaseBefore: {a,b,c}, databaseAfter: {a},     value: a, previousEntriesPolicy: 'remove' },
+	// Latest found spec key is returned AND removed from db if not current spec
+	{ spec: [{ key:a }, {key:b}],          databaseBefore: {b},     databaseAfter: {a:b},   value: b, previousEntriesPolicy: 'remove' },
+	{ spec: [{ key:a }, {key:b}, {key:c}], databaseBefore: {c},     databaseAfter: {a:c},   value: c, previousEntriesPolicy: 'remove' },
+	{ spec: [{ key:a }, {key:b}, {key:c}], databaseBefore: {b,c},   databaseAfter: {a:b},   value: b, previousEntriesPolicy: 'remove' },
+	// Non-spec keys in db are not removed
+	{ spec: [{ key:a }, {key:b}],          databaseBefore: {a,b,c}, databaseAfter: {a,c},   value: a, previousEntriesPolicy: 'remove' },
+])(
+  '%j',
+  ({ databaseBefore, databaseAfter, spec, value, previousEntriesPolicy }) => {
+    const readVersionedEntryMemory = readVersionedEntry(createKeyValueStoreMemory(databaseBefore));
+		const valueActual = readVersionedEntryMemory({spec, previousEntriesPolicy})
+		expect(databaseBefore).toEqual(databaseAfter)
+		expect(valueActual).toEqual(value)
+  },
+);