Skip to content

Commit ff33aee

Browse files
authored
fix(laboratory): Preflight Script environment variables no parse storage read (#6450)
1 parent c097974 commit ff33aee

File tree

7 files changed

+63
-20
lines changed

7 files changed

+63
-20
lines changed

cypress/e2e/laboratory-preflight-script.cy.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { dedent } from '../support/testkit';
22

33
const selectors = {
4+
buttonPreflightScript: '[aria-label*="Preflight Script"]',
45
buttonModalCy: 'preflight-script-modal-button',
56
buttonToggleCy: 'toggle-preflight-script',
67
buttonHeaders: '[data-name="headers"]',
@@ -16,13 +17,17 @@ const selectors = {
1617
},
1718
};
1819

20+
const data: { slug: string } = {
21+
slug: '',
22+
};
23+
1924
beforeEach(() => {
2025
cy.clearLocalStorage().then(async () => {
2126
cy.task('seedTarget').then(({ slug, refreshToken }: any) => {
2227
cy.setCookie('sRefreshToken', refreshToken);
23-
28+
data.slug = slug;
2429
cy.visit(`/${slug}/laboratory`);
25-
cy.get('[aria-label*="Preflight Script"]').click();
30+
cy.get(selectors.buttonPreflightScript).click();
2631
});
2732
});
2833
});
@@ -51,6 +56,12 @@ function setEditorScript(script: string) {
5156
}
5257

5358
describe('Laboratory > Preflight Script', () => {
59+
// https://github.com/graphql-hive/console/pull/6450
60+
it('regression: loads even if local storage is set to {}', () => {
61+
window.localStorage.setItem('hive:laboratory:environment', '{}');
62+
cy.visit(`/${data.slug}/laboratory`);
63+
cy.get(selectors.buttonPreflightScript).click();
64+
});
5465
it('mini script editor is read only', () => {
5566
cy.dataCy('toggle-preflight-script').click();
5667
// Wait loading disappears

packages/web/app/src/components/target/explorer/provider.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
import { startOfDay } from 'date-fns';
1111
import { resolveRange, type Period } from '@/lib/date-math';
1212
import { subDays } from '@/lib/date-time';
13-
import { useLocalStorage } from '@/lib/hooks';
13+
import { useLocalStorageJson } from '@/lib/hooks';
1414
import { UTCDate } from '@date-fns/utc';
1515

1616
type SchemaExplorerContextType = {
@@ -54,11 +54,11 @@ export function SchemaExplorerProvider({ children }: { children: ReactNode }): R
5454
[dataRetentionInDays],
5555
);
5656

57-
const [isArgumentListCollapsed, setArgumentListCollapsed] = useLocalStorage(
57+
const [isArgumentListCollapsed, setArgumentListCollapsed] = useLocalStorageJson(
5858
'hive:schema-explorer:collapsed',
5959
true,
6060
);
61-
const [period, setPeriod] = useLocalStorage<Period>(
61+
const [period, setPeriod] = useLocalStorageJson<Period>(
6262
'hive:schema-explorer:period-1',
6363
defaultPeriod,
6464
);

packages/web/app/src/components/ui/changelog/changelog.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { ReactElement, useCallback, useEffect } from 'react';
22
import { format } from 'date-fns/format';
33
import { Button } from '@/components/ui/button';
44
import { Popover, PopoverArrow, PopoverContent, PopoverTrigger } from '@/components/ui/popover';
5-
import { useLocalStorage, useToggle } from '@/lib/hooks';
5+
import { useLocalStorageJson, useToggle } from '@/lib/hooks';
66
import { cn } from '@/lib/utils';
77

88
export type Changelog = {
@@ -19,8 +19,8 @@ export function Changelog(props: { changes: Changelog[] }): ReactElement {
1919

2020
function ChangelogPopover(props: { changes: Changelog[] }) {
2121
const [isOpen, toggle] = useToggle();
22-
const [displayDot, setDisplayDot] = useLocalStorage<boolean>('hive:changelog:dot', false);
23-
const [readChanges, setReadChanges] = useLocalStorage<string[]>('hive:changelog:read', []);
22+
const [displayDot, setDisplayDot] = useLocalStorageJson<boolean>('hive:changelog:dot', false);
23+
const [readChanges, setReadChanges] = useLocalStorageJson<string[]>('hive:changelog:read', []);
2424
const hasNewChanges = props.changes.some(change => !readChanges.includes(change.href));
2525

2626
useEffect(() => {

packages/web/app/src/lib/hooks/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ export { toDecimal, useDecimal } from './use-decimal';
33
export { formatDuration, useFormattedDuration } from './use-formatted-duration';
44
export { formatNumber, useFormattedNumber } from './use-formatted-number';
55
export { formatThroughput, useFormattedThroughput } from './use-formatted-throughput';
6+
export { useLocalStorageJson } from './use-local-storage-json';
67
export { useLocalStorage } from './use-local-storage';
78
export { useNotifications } from './use-notifications';
89
export { usePrettify } from './use-prettify';
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { useCallback, useState } from 'react';
2+
3+
export function useLocalStorageJson<T>(key: string, defaultValue: T) {
4+
const [value, setValue] = useState<T>(() => {
5+
const json = localStorage.getItem(key);
6+
try {
7+
const result = json ? JSON.parse(json) : defaultValue;
8+
return result;
9+
} catch (_) {
10+
return defaultValue;
11+
}
12+
});
13+
14+
const set = useCallback(
15+
(value: T) => {
16+
localStorage.setItem(key, JSON.stringify(value));
17+
setValue(value);
18+
},
19+
[setValue],
20+
);
21+
22+
return [value, set] as const;
23+
}

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,14 @@
11
import { useCallback, useState } from 'react';
22

3-
export function useLocalStorage<T>(key: string, defaultValue: T) {
4-
const [value, setValue] = useState<T>(() => {
5-
const json = localStorage.getItem(key);
6-
try {
7-
return json ? JSON.parse(json) : defaultValue;
8-
} catch (_) {
9-
return defaultValue;
10-
}
3+
export function useLocalStorage(key: string, defaultValue: string) {
4+
const [value, setValue] = useState<string>(() => {
5+
const value = localStorage.getItem(key);
6+
return value ?? defaultValue;
117
});
128

139
const set = useCallback(
14-
(value: T) => {
15-
localStorage.setItem(key, JSON.stringify(value));
10+
(value: string) => {
11+
localStorage.setItem(key, value);
1612
setValue(value);
1713
},
1814
[setValue],

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { Subtitle } from '@/components/ui/page';
2626
import { usePromptManager } from '@/components/ui/prompt';
2727
import { useToast } from '@/components/ui/use-toast';
2828
import { FragmentType, graphql, useFragment } from '@/gql';
29-
import { useLocalStorage, useToggle } from '@/lib/hooks';
29+
import { useLocalStorage, useLocalStorageJson, useToggle } from '@/lib/hooks';
3030
import { GraphiQLPlugin } from '@graphiql/react';
3131
import { Editor as MonacoEditor, OnMount, type Monaco } from '@monaco-editor/react';
3232
import { Cross2Icon, InfoCircledIcon, Pencil1Icon, TriangleRightIcon } from '@radix-ui/react-icons';
@@ -153,7 +153,7 @@ export function usePreflightScript(args: {
153153
const prompt = usePromptManager();
154154

155155
const target = useFragment(PreflightScript_TargetFragment, args.target);
156-
const [isPreflightScriptEnabled, setIsPreflightScriptEnabled] = useLocalStorage(
156+
const [isPreflightScriptEnabled, setIsPreflightScriptEnabled] = useLocalStorageJson(
157157
'hive:laboratory:isPreflightScriptEnabled',
158158
false,
159159
);
@@ -178,6 +178,17 @@ export function usePreflightScript(args: {
178178
const resultEnvironmentVariablesDecoded: PreflightScriptResultData['environmentVariables'] =
179179
Kit.tryOr(
180180
() => JSON.parse(latestEnvironmentVariablesRef.current),
181+
// todo: find a better solution than blowing away the user's
182+
// invalid localStorage state.
183+
//
184+
// For example if the user has:
185+
//
186+
// { "foo": "bar }
187+
//
188+
// Then when they "Run Script" it will be replaced with:
189+
//
190+
// {}
191+
//
181192
() => ({}),
182193
);
183194
const result: PreflightScriptResultData = {
@@ -280,6 +291,7 @@ export function usePreflightScript(args: {
280291

281292
// Cause the new state of environment variables to be
282293
// written back to local storage.
294+
283295
const mergedEnvironmentVariablesEncoded = JSON.stringify(
284296
result.environmentVariables,
285297
null,

0 commit comments

Comments
 (0)