Skip to content

Commit

Permalink
Fix a handful of strict TS errors
Browse files Browse the repository at this point in the history
  • Loading branch information
codykaup committed Oct 1, 2024
1 parent a69bb57 commit f1cacff
Show file tree
Hide file tree
Showing 29 changed files with 98 additions and 85 deletions.
7 changes: 4 additions & 3 deletions node-src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ vi.mock('node-fetch', () => ({
// TODO: refactor this function
// eslint-disable-next-line complexity, max-statements
json: async () => {
let query: string, variables: Record<string, any>;
let query = '';
let variables: Record<string, any> = {};
try {
const data = JSON.parse(body);
query = data.query;
Expand Down Expand Up @@ -406,7 +407,7 @@ it('passes options error to experimental_onTaskError', async () => {
ctx.env.CHROMATIC_PROJECT_TOKEN = '';
await runAll(ctx);

expect(ctx.extraOptions.experimental_onTaskError).toHaveBeenCalledWith(
expect(ctx.extraOptions?.experimental_onTaskError).toHaveBeenCalledWith(
expect.anything(), // Context
expect.objectContaining({
formattedError: expect.stringContaining('Missing project token'), // Long formatted error fatalError https://github.com/chromaui/chromatic-cli/blob/217e77671179748eb4ddb8becde78444db93d067/node-src/ui/messages/errors/fatalError.ts#L11
Expand Down Expand Up @@ -764,7 +765,7 @@ it('should upload metadata files if --upload-metadata is passed', async () => {
'--only-changed',
]);
await runAll(ctx);
expect(upload.mock.calls.at(-1)[1]).toEqual(
expect(upload.mock.calls.at(-1)?.[1]).toEqual(
expect.arrayContaining([
{
contentLength: expect.any(Number),
Expand Down
2 changes: 1 addition & 1 deletion node-src/io/httpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export default class HTTPClient {

constructor(
{ env, log }: Pick<Context, 'env' | 'log'>,
{ headers, retries = 0 }: HTTPClientOptions = {}
{ headers = {}, retries = 0 }: HTTPClientOptions = {}
) {
if (!log) throw new Error(`Missing required option in HTTPClient: log`);
this.env = env;
Expand Down
8 changes: 5 additions & 3 deletions node-src/lib/findChangedDependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,20 @@ const YARN_LOCK = 'yarn.lock';

// Yields a list of dependency names which have changed since the baseline.
// E.g. ['react', 'react-dom', '@storybook/react']
// TODO: refactor this function
// eslint-disable-next-line complexity
export const findChangedDependencies = async (ctx: Context) => {
const { packageMetadataChanges } = ctx.git;
const { untraced = [] } = ctx.options;

if (packageMetadataChanges.length === 0) {
if (packageMetadataChanges?.length === 0) {
ctx.log.debug('No package metadata changed found');
return [];
}

ctx.log.debug(
{ packageMetadataChanges },
`Finding changed dependencies for ${packageMetadataChanges.length} baselines`
`Finding changed dependencies for ${packageMetadataChanges?.length} baselines`
);

const rootPath = await getRepositoryRoot();
Expand Down Expand Up @@ -69,7 +71,7 @@ export const findChangedDependencies = async (ctx: Context) => {
const filteredPathPairs = metadataPathPairs
.map(([manifestPath, lockfilePath]) => {
const commits = packageMetadataChanges
.filter(({ changedFiles }) =>
?.filter(({ changedFiles }) =>
changedFiles.some((file) => file === lockfilePath || file === manifestPath)
)
.map(({ commit }) => commit);
Expand Down
8 changes: 4 additions & 4 deletions node-src/lib/getEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ export interface Environment {
CHROMATIC_INDEX_URL: string;
CHROMATIC_OUTPUT_INTERVAL: number;
CHROMATIC_POLL_INTERVAL: number;
CHROMATIC_PROJECT_TOKEN: string;
CHROMATIC_PROJECT_TOKEN?: string;
CHROMATIC_RETRIES: number;
CHROMATIC_STORYBOOK_VERSION: string;
CHROMATIC_STORYBOOK_VERSION?: string;
CHROMATIC_TIMEOUT: number;
CHROMATIC_UPGRADE_TIMEOUT: number;
ENVIRONMENT_WHITELIST: RegExp[];
HTTP_PROXY: string;
HTTPS_PROXY: string;
HTTP_PROXY?: string;
HTTPS_PROXY?: string;
STORYBOOK_BUILD_TIMEOUT: number;
STORYBOOK_CLI_FLAGS_BY_VERSION: typeof STORYBOOK_CLI_FLAGS_BY_VERSION;
STORYBOOK_VERIFY_TIMEOUT: number;
Expand Down
4 changes: 2 additions & 2 deletions node-src/lib/getFileHashes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const hashFile = (buffer: Buffer, path: string, xxhash: XXHashAPI): Promise<stri
};

const readIncremental = (fd: number, hash: XXHash<bigint>) => {
read(fd, buffer, undefined, BUFFER_SIZE, -1, (readError, bytesRead) => {
read(fd, buffer, 0, BUFFER_SIZE, -1, (readError, bytesRead) => {
if (readError) {
return close(fd, () => reject(readError));
}
Expand All @@ -36,7 +36,7 @@ const hashFile = (buffer: Buffer, path: string, xxhash: XXHashAPI): Promise<stri
if (openError) {
return reject(openError);
}
read(fd, buffer, undefined, BUFFER_SIZE, -1, (readError, bytesRead) => {
read(fd, buffer, 0, BUFFER_SIZE, -1, (readError, bytesRead) => {
if (readError) {
return close(fd, () => reject(readError));
}
Expand Down
2 changes: 1 addition & 1 deletion node-src/lib/getOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export default function getOptions(ctx: InitialContext): Options {
debug: flags.debug,
diagnosticsFile:
defaultIfSet(flags.diagnosticsFile, DEFAULT_DIAGNOSTICS_FILE) ||
defaultIfSet(flags.diagnostics && '', DEFAULT_DIAGNOSTICS_FILE), // for backwards compatibility
defaultIfSet(flags.diagnostics?.toString(), DEFAULT_DIAGNOSTICS_FILE), // for backwards compatibility
junitReport: defaultIfSet(flags.junitReport, DEFAULT_REPORT_FILE),
zip: flags.zip,
skipUpdateCheck: flags.skipUpdateCheck,
Expand Down
8 changes: 3 additions & 5 deletions node-src/lib/installDependencies.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import { SpawnOptions } from 'child_process';
import yon from 'yarn-or-npm';

const { spawn } = yon;
import { spawn } from 'yarn-or-npm';

const installDependencies = (options?: SpawnOptions) =>
new Promise((resolve, reject) => {
let stdout = '';
let stderr = '';
const child = spawn(['install'], options);
child.stdout.on('data', (chunk) => {
child.stdout?.on('data', (chunk) => {
stdout += chunk;
});
child.stderr.on('data', (chunk) => {
child.stderr?.on('data', (chunk) => {
stderr += chunk;
});
child.on('error', reject);
Expand Down
4 changes: 2 additions & 2 deletions node-src/lib/spawn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ export default function spawn(
let stdout = '';
let stderr = '';
const child = packageCommand(args, options);
child.stdout.on('data', (chunk) => {
child.stdout?.on('data', (chunk) => {
stdout += chunk;
});
child.stderr.on('data', (chunk) => {
child.stderr?.on('data', (chunk) => {
stderr += chunk;
});
child.on('error', reject);
Expand Down
14 changes: 7 additions & 7 deletions node-src/lib/upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,17 +148,17 @@ export async function uploadBuild(
return options.onError?.(new Error('Upload rejected due to user error'));
}

ctx.sentinelUrls.push(...uploadBuild.info.sentinelUrls);
ctx.sentinelUrls.push(...(uploadBuild.info?.sentinelUrls || []));
targets.push(
...uploadBuild.info.targets.map((target) => {
...(uploadBuild.info?.targets.map((target) => {
const file = batch.find((f) => f.targetPath === target.filePath);
return { ...file, ...target };
})
return { ...file, ...target } as FileDesc & TargetInfo;
}) || [])
);

// Use the last received zipTarget, as it will have the largest allowed size.
// If all files in the batch are copied rather than uploaded, we won't receive a zipTarget.
if (uploadBuild.info.zipTarget) {
if (uploadBuild.info?.zipTarget) {
zipTarget = uploadBuild.info.zipTarget;
}
}
Expand Down Expand Up @@ -194,7 +194,7 @@ export async function uploadBuild(
} catch (err) {
const target = targets.find((target) => target.localPath === err.message);
if (target) ctx.log.error(uploadFailed({ target }, ctx.log.getLevel() === 'debug'));
return options.onError?.(err, target.localPath);
return options.onError?.(err, target?.localPath);
}
}

Expand Down Expand Up @@ -254,7 +254,7 @@ export async function uploadMetadata(ctx: Context, files: FileDesc[]) {
if (uploadMetadata.info) {
const targets = uploadMetadata.info.targets.map((target) => {
const file = files.find((f) => f.targetPath === target.filePath);
return { ...file, ...target };
return { ...file, ...target } as FileDesc & TargetInfo;
});
await uploadFiles(ctx, targets);
}
Expand Down
11 changes: 5 additions & 6 deletions node-src/lib/uploadMetadataFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,18 @@ export async function uploadMetadataFiles(ctx: Context) {
await findStorybookConfigFile(ctx, /^main\.[jt]sx?$/).catch(() => undefined),
await findStorybookConfigFile(ctx, /^preview\.[jt]sx?$/).catch(() => undefined),
ctx.fileInfo?.statsPath && (await trimStatsFile([ctx.fileInfo.statsPath])),
].filter(Boolean);
].filter((m): m is string => !!m);

const files = await Promise.all<FileDesc>(
let files = await Promise.all(
metadataFiles.map(async (localPath) => {
const contentLength = await fileSize(localPath);
const targetPath = `.chromatic/${path.basename(localPath)}`;
return contentLength && { contentLength, localPath, targetPath };
})
).then((files) =>
files
.filter(Boolean)
.sort((a, b) => a.targetPath.localeCompare(b.targetPath, 'en', { numeric: true }))
);
files = files
.filter((f): f is FileDesc => !!f)
.sort((a, b) => a.targetPath.localeCompare(b.targetPath, 'en', { numeric: true }));

if (files.length === 0) {
ctx.log.warn('No metadata files found, skipping metadata upload.');
Expand Down
4 changes: 2 additions & 2 deletions node-src/lib/waitForSentinel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ export async function waitForSentinel(ctx: Context, { name, url }: { name: strin
if (response.status === 404) {
throw new Error(`Sentinel file '${name}' not present.`);
}
if (this.log.getLevel() === 'debug') {
this.log.debug(await response.text());
if (ctx.log.getLevel() === 'debug') {
ctx.log.debug(await response.text());
}
return bail(new Error(message));
}
Expand Down
2 changes: 1 addition & 1 deletion node-src/tasks/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const setAuthorizationToken = async (ctx: Context) => {
} catch (errors) {
const message = errors[0]?.message;
if (message?.match('Must login') || message?.match('No Access')) {
throw new Error(invalidProjectId({ projectId: ctx.options.projectId }));
throw new Error(invalidProjectId({ projectId: ctx.options.projectId || '' }));
}
if (message?.match('No app with code')) {
throw new Error(invalidProjectToken({ projectToken: ctx.options.projectToken }));
Expand Down
4 changes: 2 additions & 2 deletions node-src/tasks/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export const setSourceDirectory = async (ctx: Context) => {
export const setBuildCommand = async (ctx: Context) => {
const webpackStatsSupported =
ctx.storybook && ctx.storybook.version
? semver.gte(semver.coerce(ctx.storybook.version), '6.2.0')
? semver.gte(semver.coerce(ctx.storybook.version) || '', '6.2.0')
: true;

if (ctx.git.changedFiles && !webpackStatsSupported) {
Expand All @@ -40,7 +40,7 @@ export const setBuildCommand = async (ctx: Context) => {
const buildCommandOptions = [
`--output-dir=${ctx.sourceDir}`,
ctx.git.changedFiles && webpackStatsSupported && `--webpack-stats-json=${ctx.sourceDir}`,
].filter(Boolean);
].filter((c): c is string => !!c);

ctx.buildCommand = await (isE2EBuild(ctx.options)
? getE2EBuildCommand(
Expand Down
6 changes: 3 additions & 3 deletions node-src/tasks/gitInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ export const setGitInfo = async (ctx: Context, task: Task) => {

const { branch, commit, slug } = ctx.git;

ctx.git.matchesBranch = (glob: true | string) =>
ctx.git.matchesBranch = (glob: string | boolean) =>
typeof glob === 'string' && glob.length > 0 ? picomatch(glob, { bash: true })(branch) : !!glob;

if (ctx.git.matchesBranch(ctx.options.skip)) {
if (ctx.git.matchesBranch?.(ctx.options.skip)) {
transitionTo(skippingBuild)(ctx, task);
// The SkipBuildMutation ensures the commit is tagged properly.
if (await ctx.client.runQuery(SkipBuildMutation, { commit, branch, slug })) {
Expand All @@ -119,7 +119,7 @@ export const setGitInfo = async (ctx: Context, task: Task) => {
}

const parentCommits = await getParentCommits(ctx, {
ignoreLastBuildOnBranch: ctx.git.matchesBranch(ctx.options.ignoreLastBuildOnBranch),
ignoreLastBuildOnBranch: ctx.git.matchesBranch?.(ctx.options.ignoreLastBuildOnBranch || false),
});
ctx.git.parentCommits = parentCommits;
ctx.log.debug(`Found parentCommits: ${parentCommits.join(', ')}`);
Expand Down
2 changes: 1 addition & 1 deletion node-src/tasks/initialize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const announceBuild = async (ctx: Context) => {
...commitInfo
} = ctx.git; // omit some fields;
const { rebuildForBuildId, turboSnap } = ctx;
const autoAcceptChanges = matchesBranch(ctx.options.autoAcceptChanges);
const autoAcceptChanges = matchesBranch?.(ctx.options.autoAcceptChanges);

const { announceBuild: announcedBuild } = await ctx.client.runQuery<AnnounceBuildMutationResult>(
AnnounceBuildMutation,
Expand Down
7 changes: 5 additions & 2 deletions node-src/tasks/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export const takeSnapshots = async (ctx: Context, task: Task) => {
const testLabels =
ctx.options.interactive &&
testCount === actualTestCount &&
tests.map(({ spec, parameters, mode }) => {
tests?.map(({ spec, parameters, mode }) => {
const testSuffixName = mode.name || `[${parameters.viewport}px]`;
const suffix = parameters.viewportIsDefault ? '' : testSuffixName;
return `${spec.component.displayName}${spec.name} ${suffix}`;
Expand Down Expand Up @@ -113,7 +113,10 @@ export const takeSnapshots = async (ctx: Context, task: Task) => {
case 'ACCEPTED':
case 'PENDING':
case 'DENIED': {
if (build.autoAcceptChanges || ctx.git.matchesBranch(ctx.options.exitZeroOnChanges)) {
if (
build.autoAcceptChanges ||
ctx.git.matchesBranch?.(ctx.options?.exitZeroOnChanges || false)
) {
setExitCode(ctx, exitCodes.OK);
ctx.log.info(buildPassedMessage(ctx));
} else {
Expand Down
2 changes: 1 addition & 1 deletion node-src/tasks/upload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ describe('traceChangedFiles', () => {
findChangedDependencies.mockReset();
findChangedPackageFiles.mockReset();
getDependentStoryFiles.mockReset();
accessMock.mockImplementation((_path, callback) => Promise.resolve(callback(undefined)));
accessMock.mockImplementation((_path, callback) => Promise.resolve(callback(null)));
});

it('sets onlyStoryFiles on context', async () => {
Expand Down
16 changes: 8 additions & 8 deletions node-src/tasks/upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ function getFileInfo(ctx: Context, sourceDirectory: string) {
}));
const total = lengths.map(({ contentLength }) => contentLength).reduce((a, b) => a + b, 0);
const paths: string[] = [];
let statsPath: string;
let statsPath = '';
for (const { knownAs } of lengths) {
if (knownAs.endsWith('preview-stats.json')) statsPath = path.join(sourceDirectory, knownAs);
else if (!knownAs.endsWith('manager-stats.json')) paths.push(knownAs);
Expand Down Expand Up @@ -119,10 +119,10 @@ export const validateFiles = async (ctx: Context) => {
export const traceChangedFiles = async (ctx: Context, task: Task) => {
if (!ctx.turboSnap || ctx.turboSnap.unavailable) return;
if (!ctx.git.changedFiles) return;
if (!ctx.fileInfo.statsPath) {
if (!ctx.fileInfo?.statsPath) {
// If we don't know the SB version, we should assume we don't support `--stats-json`
const nonLegacyStatsSupported =
ctx.storybook?.version && semver.gte(semver.coerce(ctx.storybook.version), '8.0.0');
ctx.storybook?.version && semver.gte(semver.coerce(ctx.storybook.version) || '', '8.0.0');

ctx.turboSnap.bailReason = { missingStatsFile: true };
throw new Error(missingStatsFile({ legacy: !nonLegacyStatsSupported }));
Expand All @@ -136,7 +136,7 @@ export const traceChangedFiles = async (ctx: Context, task: Task) => {
try {
// eslint-disable-next-line @typescript-eslint/no-invalid-void-type
let changedDependencyNames: void | string[] = [];
if (packageMetadataChanges?.length > 0) {
if (packageMetadataChanges?.length) {
changedDependencyNames = await findChangedDependencies(ctx).catch((err) => {
const { name, message, stack, code } = err;
ctx.log.debug({ name, message, stack, code });
Expand Down Expand Up @@ -229,9 +229,9 @@ export const uploadStorybook = async (ctx: Context, task: Task) => {
if (ctx.skip) return;
transitionTo(starting)(ctx, task);

const files = ctx.fileInfo.paths.map<FileDesc>((filePath) => ({
...(ctx.fileInfo.hashes && { contentHash: ctx.fileInfo.hashes[filePath] }),
contentLength: ctx.fileInfo.lengths.find(({ knownAs }) => knownAs === filePath).contentLength,
const files = ctx.fileInfo?.paths.map<FileDesc>((filePath) => ({
...(ctx.fileInfo?.hashes && { contentHash: ctx.fileInfo.hashes[filePath] }),
contentLength: ctx.fileInfo?.lengths.find(({ knownAs }) => knownAs === filePath)?.contentLength,
localPath: path.join(ctx.sourceDir, filePath),
targetPath: filePath,
}));
Expand Down Expand Up @@ -260,7 +260,7 @@ export const waitForSentinels = async (ctx: Context, task: Task) => {
const sentinels = Object.fromEntries(
ctx.sentinelUrls.map((url) => {
const { host, pathname } = new URL(url);
return [host + pathname, { name: pathname.split('/').at(-1), url }];
return [host + pathname, { name: pathname.split('/').at(-1) || '', url }];
})
);

Expand Down
8 changes: 4 additions & 4 deletions node-src/tasks/verify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,15 +251,15 @@ export const verifyBuild = async (ctx: Context, task: Task) => {

if (ctx.build.wasLimited) {
const { account } = ctx.build.app;
if (account.exceededThreshold) {
if (account?.exceededThreshold) {
ctx.log.warn(snapshotQuotaReached(account));
setExitCode(ctx, exitCodes.ACCOUNT_QUOTA_REACHED, true);
} else if (account.paymentRequired) {
} else if (account?.paymentRequired) {
ctx.log.warn(paymentRequired(account));
setExitCode(ctx, exitCodes.ACCOUNT_PAYMENT_REQUIRED, true);
} else {
// Future proofing for reasons we aren't aware of
ctx.log.warn(buildLimited(account));
if (account) ctx.log.warn(buildLimited(account));
setExitCode(ctx, exitCodes.BUILD_WAS_LIMITED, true);
}
}
Expand All @@ -268,7 +268,7 @@ export const verifyBuild = async (ctx: Context, task: Task) => {

transitionTo(success, true)(ctx, task);

if (list || ctx.isPublishOnly || matchesBranch(ctx.options.exitOnceUploaded)) {
if (list || ctx.isPublishOnly || matchesBranch?.(ctx.options.exitOnceUploaded)) {
setExitCode(ctx, exitCodes.OK);
ctx.skipSnapshots = true;
}
Expand Down
Loading

0 comments on commit f1cacff

Please sign in to comment.