Skip to content

Commit

Permalink
Merge pull request #110 from chromaui/tom/strict-types
Browse files Browse the repository at this point in the history
Turn on strict types (!)
  • Loading branch information
tmeasday authored Sep 18, 2023
2 parents db9f2b8 + 4594b11 commit 29085ad
Show file tree
Hide file tree
Showing 40 changed files with 372 additions and 262 deletions.
8 changes: 8 additions & 0 deletions src/Panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ export const Panel = ({ active, api }: PanelProps) => {
);

if (projectUpdatingFailed) {
// These should always be set when we get this error
if (!projectToken || !configFile) {
throw new Error(`Missing projectToken/config file after configuration failure`);
}

return (
<Sections hidden={!active}>
<LinkingProjectFailed
Expand All @@ -86,6 +91,9 @@ export const Panel = ({ active, api }: PanelProps) => {
}

if (projectIdUpdated) {
// This should always be set when we succeed
if (!configFile) throw new Error(`Missing config file after configuration success`);

return (
<Provider key={PANEL_ID} value={client}>
<Sections hidden={!active}>
Expand Down
3 changes: 3 additions & 0 deletions src/SidebarTop.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const SidebarTop = ({ api }: SidebarTopProps) => {
name: "passed",
color: color.positive,
},
// @ts-expect-error SB needs a proper API for no link
link: undefined,
});
setTimeout(() => clearNotification(`${ADDON_ID}/build-initialize`), 10_000);
Expand All @@ -62,6 +63,7 @@ export const SidebarTop = ({ api }: SidebarTopProps) => {
name: "passed",
color: color.positive,
},
// @ts-expect-error SB needs a proper API for no link
link: undefined,
});
setTimeout(() => clearNotification(`${ADDON_ID}/build-complete`), 10_000);
Expand All @@ -78,6 +80,7 @@ export const SidebarTop = ({ api }: SidebarTopProps) => {
name: "failed",
color: color.negative,
},
// @ts-expect-error SB needs a proper API for no link
link: undefined,
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/buildSteps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export const BUILD_STEP_CONFIG: Record<
renderName: () => `Publish your Storybook`,
renderProgress: ({ stepProgress }) => {
const { numerator, denominator } = stepProgress.upload;
if (!denominator) return `Uploading files`;
if (!denominator || !numerator) return `Uploading files`;
const { value: total, exponent } = filesize(denominator, {
output: "object",
round: 1,
Expand Down
5 changes: 2 additions & 3 deletions src/components/BrowserSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type BrowserData = Pick<BrowserInfo, "id" | "key" | "name">;
interface BrowserSelectorProps {
isAccepted: boolean;
selectedBrowser: BrowserData;
browserResults: { browser: BrowserData; result: ComparisonResult }[];
browserResults: { browser: BrowserData; result?: ComparisonResult }[];
onSelectBrowser: (browser: BrowserData) => void;
}

Expand All @@ -52,7 +52,7 @@ export const BrowserSelector = ({

type Link = ComponentProps<typeof TooltipMenu>["links"][0];

const links: Link[] =
const links =
browserResults.length > 1 &&
browserResults.map(
({ browser, result }): Link => ({
Expand All @@ -64,7 +64,6 @@ export const BrowserSelector = ({
title: browser.name,
})
);

return (
<WithTooltip
hasChrome={false}
Expand Down
2 changes: 1 addition & 1 deletion src/components/ModeSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ interface ModeSelectorProps {
isAccepted: boolean;
selectedMode: ModeData;
onSelectMode: (viewport: ModeData) => void;
modeResults: { viewport: ModeData; result: ComparisonResult }[];
modeResults: { viewport: ModeData; result?: ComparisonResult }[];
}

export const ModeSelector = ({
Expand Down
2 changes: 1 addition & 1 deletion src/components/SnapshotImage.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export const TallerConstrained: Story = {

export const CaptureError: Story = {
args: {
captureImage: null,
captureImage: undefined,
comparisonResult: ComparisonResult.CaptureError,
},
};
6 changes: 3 additions & 3 deletions src/components/SnapshotImage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ export const Container = styled.div<{ href?: string; target?: string }>(
);

interface SnapshotImageProps {
componentName: Test["story"]["component"]["name"];
storyName: Test["story"]["name"];
componentName?: NonNullable<NonNullable<Test["story"]>["component"]>["name"];
storyName?: NonNullable<Test["story"]>["name"];
testUrl: Test["webUrl"];
comparisonResult: ComparisonResult;
comparisonResult?: ComparisonResult;
captureImage?: Pick<CaptureImage, "imageUrl" | "imageWidth">;
diffImage?: Pick<CaptureOverlayImage, "imageUrl" | "imageWidth">;
focusImage?: Pick<CaptureOverlayImage, "imageUrl" | "imageWidth">;
Expand Down
36 changes: 19 additions & 17 deletions src/components/StatusDot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,25 @@ const Dot = styled.div<StatusDotProps & { overlay?: boolean }>(
width: 6,
height: 6,
borderRadius: "50%",
background: {
[TestStatus.InProgress]: "transparent",
[TestStatus.Passed]: theme.color.positive,
[TestStatus.Pending]: theme.color.gold,
[TestStatus.Accepted]: theme.color.positive,
[TestStatus.Denied]: theme.color.positive,
[TestStatus.Broken]: theme.color.negative,
[TestStatus.Failed]: theme.color.negative,
[ComparisonResult.Equal]: theme.color.positive,
[ComparisonResult.Fixed]: theme.color.positive,
[ComparisonResult.Added]: theme.color.gold,
[ComparisonResult.Changed]: theme.color.gold,
[ComparisonResult.Removed]: theme.color.gold,
[ComparisonResult.CaptureError]: theme.color.negative,
[ComparisonResult.SystemError]: theme.color.negative,
notification: theme.color.secondary,
}[status],
background:
status &&
{
[TestStatus.InProgress]: "transparent",
[TestStatus.Passed]: theme.color.positive,
[TestStatus.Pending]: theme.color.gold,
[TestStatus.Accepted]: theme.color.positive,
[TestStatus.Denied]: theme.color.positive,
[TestStatus.Broken]: theme.color.negative,
[TestStatus.Failed]: theme.color.negative,
[ComparisonResult.Equal]: theme.color.positive,
[ComparisonResult.Fixed]: theme.color.positive,
[ComparisonResult.Added]: theme.color.gold,
[ComparisonResult.Changed]: theme.color.gold,
[ComparisonResult.Removed]: theme.color.gold,
[ComparisonResult.CaptureError]: theme.color.negative,
[ComparisonResult.SystemError]: theme.color.negative,
notification: theme.color.secondary,
}[status],
}),
({ overlay, theme }) =>
overlay &&
Expand Down
4 changes: 2 additions & 2 deletions src/components/SuffixInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ const SuffixOverlay = ({
placeholder,
suffix,
}: {
value: string;
placeholder: string;
value?: string;
placeholder?: string;
suffix: ReactNode;
}) => (
<SuffixWrapper>
Expand Down
2 changes: 1 addition & 1 deletion src/components/TooltipMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const TooltipMenu = ({ children, links, note, ...props }: TooltipMenuProp
...link,
onClick: (...args) => {
onHide();
return link.onClick(...args);
return link.onClick?.(...args);
},
}))}
/>
Expand Down
4 changes: 2 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const observeGitInfo = async (
callback: (info: GitInfo, prevInfo: GitInfo) => void
) => {
let prev: GitInfo;
let timer: NodeJS.Timeout | null = null;
let timer: NodeJS.Timeout | undefined;
const act = async () => {
const gitInfo = await getGitInfo();
if (Object.entries(gitInfo).some(([key, value]) => prev?.[key as keyof GitInfo] !== value)) {
Expand Down Expand Up @@ -93,7 +93,7 @@ async function serverChannel(
const localBuildProgress = useAddonState<LocalBuildProgress>(channel, LOCAL_BUILD_PROGRESS);

channel.on(START_BUILD, async () => {
const { projectToken } = projectInfoState.value;
const { projectToken } = projectInfoState.value || {};
await runChromaticBuild(localBuildProgress, { projectToken });
});

Expand Down
2 changes: 1 addition & 1 deletion src/manager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ addons.register(ADDON_ID, (api) => {
type: types.PANEL,
title: "Visual Tests",
match: ({ viewMode }) => viewMode === "story",
render: ({ active }) => <Panel active={active} api={api} />,
render: ({ active }) => <Panel active={!!active} api={api} />,
});

addons.add(SIDEBAR_TOP_ID, {
Expand Down
38 changes: 21 additions & 17 deletions src/runChromaticBuild.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import { INITIAL_BUILD_PAYLOAD } from "./buildSteps";
import { onCompleteOrError, onStartOrProgress, runChromaticBuild } from "./runChromaticBuild";
import { LocalBuildProgress } from "./types";

jest.mock("chromatic/node", () => ({ run: jest.fn() }));

const store = {
state: undefined as LocalBuildProgress | undefined,
get value() {
if (!this.state) throw new Error("need to call set first");

return this.state;
},
set value(newValue) {
set value(newValue: LocalBuildProgress) {
this.state = newValue;
},
on() {},
Expand Down Expand Up @@ -39,28 +43,28 @@ describe("onStartOrProgress", () => {
currentStep: "initialize",
});

onStartOrProgress(store, null)({ task: "build" } as any);
onStartOrProgress(store)({ task: "build" } as any);
expect(store.value).toMatchObject({
buildProgressPercentage: expect.closeTo(3, 0),
currentStep: "build",
stepProgress: { build: { startedAt: expect.any(Number) } },
});

onStartOrProgress(store, null)({ task: "upload" } as any);
onStartOrProgress(store)({ task: "upload" } as any);
expect(store.value).toMatchObject({
buildProgressPercentage: expect.closeTo(24, 0),
currentStep: "upload",
stepProgress: { upload: { startedAt: expect.any(Number) } },
});

onStartOrProgress(store, null)({ task: "verify" } as any);
onStartOrProgress(store)({ task: "verify" } as any);
expect(store.value).toMatchObject({
buildProgressPercentage: expect.closeTo(48, 0),
currentStep: "verify",
stepProgress: { verify: { startedAt: expect.any(Number) } },
});

onStartOrProgress(store, null)({ task: "snapshot" } as any);
onStartOrProgress(store)({ task: "snapshot" } as any);
expect(store.value).toMatchObject({
buildProgressPercentage: expect.closeTo(55, 0),
currentStep: "snapshot",
Expand All @@ -69,36 +73,36 @@ describe("onStartOrProgress", () => {
});

it("updates progress with each invocation", () => {
onStartOrProgress(store, null)({ task: "verify" } as any);
onStartOrProgress(store)({ task: "verify" } as any);
expect(store.value.buildProgressPercentage).toBeCloseTo(48, 0);

onStartOrProgress(store, null)({ task: "verify" } as any);
onStartOrProgress(store)({ task: "verify" } as any);
expect(store.value.buildProgressPercentage).toBeCloseTo(50, 0);

onStartOrProgress(store, null)({ task: "verify" } as any);
onStartOrProgress(store)({ task: "verify" } as any);
expect(store.value.buildProgressPercentage).toBeCloseTo(52, 0);
});

it("can never exceed progress for a step beyond the next step", () => {
for (let n = 10; n; n -= 1) onStartOrProgress(store, null)({ task: "verify" } as any);
for (let n = 10; n; n -= 1) onStartOrProgress(store)({ task: "verify" } as any);
expect(store.value.buildProgressPercentage).toBeCloseTo(55, 0);

for (let n = 10; n; n -= 1) onStartOrProgress(store, null)({ task: "verify" } as any);
for (let n = 10; n; n -= 1) onStartOrProgress(store)({ task: "verify" } as any);
expect(store.value.buildProgressPercentage).toBeCloseTo(55, 0);

onStartOrProgress(store, null)({ task: "snapshot" } as any);
onStartOrProgress(store)({ task: "snapshot" } as any);
expect(store.value.buildProgressPercentage).toBeCloseTo(55, 0);
});

it('updates build progress based on "progress" and "total" values', () => {
onStartOrProgress(store, null)({ task: "snapshot" } as any);
onStartOrProgress(store)({ task: "snapshot" } as any);
expect(store.value.buildProgressPercentage).toBeCloseTo(55, 0);

onStartOrProgress(store, null)({ task: "snapshot" } as any, { progress: 1, total: 2 });
onStartOrProgress(store)({ task: "snapshot" } as any, { progress: 1, total: 2 });
expect(store.value.stepProgress.snapshot).toMatchObject({ numerator: 1, denominator: 2 });
expect(store.value.buildProgressPercentage).toBeCloseTo(77, 0);

onStartOrProgress(store, null)({ task: "snapshot" } as any, { progress: 2, total: 2 });
onStartOrProgress(store)({ task: "snapshot" } as any, { progress: 2, total: 2 });
expect(store.value.stepProgress.snapshot).toMatchObject({ numerator: 2, denominator: 2 });
expect(store.value.buildProgressPercentage).toBeCloseTo(100, 0);
});
Expand All @@ -111,7 +115,7 @@ describe("onCompleteOrError", () => {

it("sets build progress to 100% on completion of final step", () => {
const build = { changeCount: 1, errorCount: 2 };
onCompleteOrError(store, null)({ task: "snapshot", build } as any);
onCompleteOrError(store)({ task: "snapshot", build } as any);
expect(store.value).toMatchObject({
buildProgressPercentage: 100,
currentStep: "complete",
Expand All @@ -122,11 +126,11 @@ describe("onCompleteOrError", () => {
});

it("does not set build progress to 100% on error", () => {
onStartOrProgress(store, null)({ task: "snapshot" } as any);
onStartOrProgress(store)({ task: "snapshot" } as any);
expect(store.value.buildProgressPercentage).toBeCloseTo(55, 0);

const error = { formattedError: "Oops!", originalError: new Error("Oops!") };
onCompleteOrError(store, null)({ task: "snapshot" } as any, error);
onCompleteOrError(store)({ task: "snapshot" } as any, error);
expect(store.value).toMatchObject({
buildProgressPercentage: expect.closeTo(55, 0),
currentStep: "error",
Expand Down
21 changes: 18 additions & 3 deletions src/runChromaticBuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,18 @@ const getBuildStepData = (
export const onStartOrProgress =
(
localBuildProgress: ReturnType<typeof useAddonState<LocalBuildProgress>>,
timeout: ReturnType<typeof setTimeout>
timeout?: ReturnType<typeof setTimeout>
) =>
({ ...ctx }: Context, { progress, total }: { progress?: number; total?: number } = {}) => {
clearTimeout(timeout);

if (!isKnownStep(ctx.task)) return;

// We should set this right before starting so it should never be unset during a build.
if (!localBuildProgress.value) {
throw new Error("Unexpected missing value for localBuildProgress");
}

const { buildProgressPercentage, stepProgress, previousBuildProgress } =
localBuildProgress.value;

Expand All @@ -74,6 +79,11 @@ export const onStartOrProgress =
(ESTIMATED_PROGRESS_INTERVAL / estimateDuration) * stepPercentage;

timeout = setTimeout(() => {
// We should set this right before starting so it should never be unset during a build.
if (!localBuildProgress.value) {
throw new Error("Unexpected missing value for localBuildProgress");
}

// Intentionally reference the present value here (after timeout)
const { currentStep } = localBuildProgress.value;
// Only update if we haven't moved on to a later step
Expand All @@ -100,11 +110,16 @@ export const onStartOrProgress =
export const onCompleteOrError =
(
localBuildProgress: ReturnType<typeof useAddonState<LocalBuildProgress>>,
timeout: ReturnType<typeof setTimeout>
timeout?: ReturnType<typeof setTimeout>
) =>
(ctx: Context, error?: { formattedError: string; originalError: Error | Error[] }) => {
clearTimeout(timeout);

// We should set this right before starting so it should never be unset during a build.
if (!localBuildProgress.value) {
throw new Error("Unexpected missing value for localBuildProgress");
}

const { buildProgressPercentage, stepProgress } = localBuildProgress.value;

if (isKnownStep(ctx.task)) {
Expand Down Expand Up @@ -149,7 +164,7 @@ export const runChromaticBuild = async (
localBuildProgress.value = INITIAL_BUILD_PAYLOAD;

// Timeout is defined here so it's shared between all handlers
let timeout: ReturnType<typeof setTimeout>;
let timeout: ReturnType<typeof setTimeout> | undefined;

await run({
// Currently we have to have these flags.
Expand Down
Loading

0 comments on commit 29085ad

Please sign in to comment.