From ff33aeed3c0b0e21018aef2c8d3f399ca29c19ce Mon Sep 17 00:00:00 2001 From: Jason Kuhrt Date: Fri, 31 Jan 2025 08:53:35 -0500 Subject: [PATCH] fix(laboratory): Preflight Script environment variables no parse storage read (#6450) --- cypress/e2e/laboratory-preflight-script.cy.ts | 15 ++++++++++-- .../components/target/explorer/provider.tsx | 6 ++--- .../src/components/ui/changelog/changelog.tsx | 6 ++--- packages/web/app/src/lib/hooks/index.ts | 1 + .../src/lib/hooks/use-local-storage-json.ts | 23 +++++++++++++++++++ .../app/src/lib/hooks/use-local-storage.ts | 16 +++++-------- .../lib/preflight-sandbox/graphiql-plugin.tsx | 16 +++++++++++-- 7 files changed, 63 insertions(+), 20 deletions(-) create mode 100644 packages/web/app/src/lib/hooks/use-local-storage-json.ts diff --git a/cypress/e2e/laboratory-preflight-script.cy.ts b/cypress/e2e/laboratory-preflight-script.cy.ts index c935988df..c278a6ec2 100644 --- a/cypress/e2e/laboratory-preflight-script.cy.ts +++ b/cypress/e2e/laboratory-preflight-script.cy.ts @@ -1,6 +1,7 @@ import { dedent } from '../support/testkit'; const selectors = { + buttonPreflightScript: '[aria-label*="Preflight Script"]', buttonModalCy: 'preflight-script-modal-button', buttonToggleCy: 'toggle-preflight-script', buttonHeaders: '[data-name="headers"]', @@ -16,13 +17,17 @@ const selectors = { }, }; +const data: { slug: string } = { + slug: '', +}; + beforeEach(() => { cy.clearLocalStorage().then(async () => { cy.task('seedTarget').then(({ slug, refreshToken }: any) => { cy.setCookie('sRefreshToken', refreshToken); - + data.slug = slug; cy.visit(`/${slug}/laboratory`); - cy.get('[aria-label*="Preflight Script"]').click(); + cy.get(selectors.buttonPreflightScript).click(); }); }); }); @@ -51,6 +56,12 @@ function setEditorScript(script: string) { } describe('Laboratory > Preflight Script', () => { + // https://github.com/graphql-hive/console/pull/6450 + it('regression: loads even if local storage is set to {}', () => { + window.localStorage.setItem('hive:laboratory:environment', '{}'); + cy.visit(`/${data.slug}/laboratory`); + cy.get(selectors.buttonPreflightScript).click(); + }); it('mini script editor is read only', () => { cy.dataCy('toggle-preflight-script').click(); // Wait loading disappears diff --git a/packages/web/app/src/components/target/explorer/provider.tsx b/packages/web/app/src/components/target/explorer/provider.tsx index 0f20373e1..dc98e836b 100644 --- a/packages/web/app/src/components/target/explorer/provider.tsx +++ b/packages/web/app/src/components/target/explorer/provider.tsx @@ -10,7 +10,7 @@ import { import { startOfDay } from 'date-fns'; import { resolveRange, type Period } from '@/lib/date-math'; import { subDays } from '@/lib/date-time'; -import { useLocalStorage } from '@/lib/hooks'; +import { useLocalStorageJson } from '@/lib/hooks'; import { UTCDate } from '@date-fns/utc'; type SchemaExplorerContextType = { @@ -54,11 +54,11 @@ export function SchemaExplorerProvider({ children }: { children: ReactNode }): R [dataRetentionInDays], ); - const [isArgumentListCollapsed, setArgumentListCollapsed] = useLocalStorage( + const [isArgumentListCollapsed, setArgumentListCollapsed] = useLocalStorageJson( 'hive:schema-explorer:collapsed', true, ); - const [period, setPeriod] = useLocalStorage( + const [period, setPeriod] = useLocalStorageJson( 'hive:schema-explorer:period-1', defaultPeriod, ); diff --git a/packages/web/app/src/components/ui/changelog/changelog.tsx b/packages/web/app/src/components/ui/changelog/changelog.tsx index 8bf8fe691..2ccd9aaf0 100644 --- a/packages/web/app/src/components/ui/changelog/changelog.tsx +++ b/packages/web/app/src/components/ui/changelog/changelog.tsx @@ -2,7 +2,7 @@ import { ReactElement, useCallback, useEffect } from 'react'; import { format } from 'date-fns/format'; import { Button } from '@/components/ui/button'; import { Popover, PopoverArrow, PopoverContent, PopoverTrigger } from '@/components/ui/popover'; -import { useLocalStorage, useToggle } from '@/lib/hooks'; +import { useLocalStorageJson, useToggle } from '@/lib/hooks'; import { cn } from '@/lib/utils'; export type Changelog = { @@ -19,8 +19,8 @@ export function Changelog(props: { changes: Changelog[] }): ReactElement { function ChangelogPopover(props: { changes: Changelog[] }) { const [isOpen, toggle] = useToggle(); - const [displayDot, setDisplayDot] = useLocalStorage('hive:changelog:dot', false); - const [readChanges, setReadChanges] = useLocalStorage('hive:changelog:read', []); + const [displayDot, setDisplayDot] = useLocalStorageJson('hive:changelog:dot', false); + const [readChanges, setReadChanges] = useLocalStorageJson('hive:changelog:read', []); const hasNewChanges = props.changes.some(change => !readChanges.includes(change.href)); useEffect(() => { diff --git a/packages/web/app/src/lib/hooks/index.ts b/packages/web/app/src/lib/hooks/index.ts index a10963c1c..3b24f52e3 100644 --- a/packages/web/app/src/lib/hooks/index.ts +++ b/packages/web/app/src/lib/hooks/index.ts @@ -3,6 +3,7 @@ export { toDecimal, useDecimal } from './use-decimal'; export { formatDuration, useFormattedDuration } from './use-formatted-duration'; export { formatNumber, useFormattedNumber } from './use-formatted-number'; export { formatThroughput, useFormattedThroughput } from './use-formatted-throughput'; +export { useLocalStorageJson } from './use-local-storage-json'; export { useLocalStorage } from './use-local-storage'; export { useNotifications } from './use-notifications'; export { usePrettify } from './use-prettify'; diff --git a/packages/web/app/src/lib/hooks/use-local-storage-json.ts b/packages/web/app/src/lib/hooks/use-local-storage-json.ts new file mode 100644 index 000000000..54c99c04b --- /dev/null +++ b/packages/web/app/src/lib/hooks/use-local-storage-json.ts @@ -0,0 +1,23 @@ +import { useCallback, useState } from 'react'; + +export function useLocalStorageJson(key: string, defaultValue: T) { + const [value, setValue] = useState(() => { + const json = localStorage.getItem(key); + try { + const result = json ? JSON.parse(json) : defaultValue; + return result; + } catch (_) { + return defaultValue; + } + }); + + const set = useCallback( + (value: T) => { + localStorage.setItem(key, JSON.stringify(value)); + setValue(value); + }, + [setValue], + ); + + return [value, set] as const; +} diff --git a/packages/web/app/src/lib/hooks/use-local-storage.ts b/packages/web/app/src/lib/hooks/use-local-storage.ts index a2e0d3803..db38bae85 100644 --- a/packages/web/app/src/lib/hooks/use-local-storage.ts +++ b/packages/web/app/src/lib/hooks/use-local-storage.ts @@ -1,18 +1,14 @@ import { useCallback, useState } from 'react'; -export function useLocalStorage(key: string, defaultValue: T) { - const [value, setValue] = useState(() => { - const json = localStorage.getItem(key); - try { - return json ? JSON.parse(json) : defaultValue; - } catch (_) { - return defaultValue; - } +export function useLocalStorage(key: string, defaultValue: string) { + const [value, setValue] = useState(() => { + const value = localStorage.getItem(key); + return value ?? defaultValue; }); const set = useCallback( - (value: T) => { - localStorage.setItem(key, JSON.stringify(value)); + (value: string) => { + localStorage.setItem(key, value); setValue(value); }, [setValue], diff --git a/packages/web/app/src/lib/preflight-sandbox/graphiql-plugin.tsx b/packages/web/app/src/lib/preflight-sandbox/graphiql-plugin.tsx index 307ed7058..b4eca68b7 100644 --- a/packages/web/app/src/lib/preflight-sandbox/graphiql-plugin.tsx +++ b/packages/web/app/src/lib/preflight-sandbox/graphiql-plugin.tsx @@ -26,7 +26,7 @@ import { Subtitle } from '@/components/ui/page'; import { usePromptManager } from '@/components/ui/prompt'; import { useToast } from '@/components/ui/use-toast'; import { FragmentType, graphql, useFragment } from '@/gql'; -import { useLocalStorage, useToggle } from '@/lib/hooks'; +import { useLocalStorage, useLocalStorageJson, useToggle } from '@/lib/hooks'; import { GraphiQLPlugin } from '@graphiql/react'; import { Editor as MonacoEditor, OnMount, type Monaco } from '@monaco-editor/react'; import { Cross2Icon, InfoCircledIcon, Pencil1Icon, TriangleRightIcon } from '@radix-ui/react-icons'; @@ -153,7 +153,7 @@ export function usePreflightScript(args: { const prompt = usePromptManager(); const target = useFragment(PreflightScript_TargetFragment, args.target); - const [isPreflightScriptEnabled, setIsPreflightScriptEnabled] = useLocalStorage( + const [isPreflightScriptEnabled, setIsPreflightScriptEnabled] = useLocalStorageJson( 'hive:laboratory:isPreflightScriptEnabled', false, ); @@ -178,6 +178,17 @@ export function usePreflightScript(args: { const resultEnvironmentVariablesDecoded: PreflightScriptResultData['environmentVariables'] = Kit.tryOr( () => JSON.parse(latestEnvironmentVariablesRef.current), + // todo: find a better solution than blowing away the user's + // invalid localStorage state. + // + // For example if the user has: + // + // { "foo": "bar } + // + // Then when they "Run Script" it will be replaced with: + // + // {} + // () => ({}), ); const result: PreflightScriptResultData = { @@ -280,6 +291,7 @@ export function usePreflightScript(args: { // Cause the new state of environment variables to be // written back to local storage. + const mergedEnvironmentVariablesEncoded = JSON.stringify( result.environmentVariables, null,