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 2892fba
Show file tree
Hide file tree
Showing 39 changed files with 151 additions and 115 deletions.
2 changes: 1 addition & 1 deletion node-src/git/getChangedFilesWithReplacement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Context } from '../types';
import { findAncestorBuildWithCommit } from './findAncestorBuildWithCommit';
import { getChangedFiles } from './git';

interface BuildWithCommitInfo {
export interface BuildWithCommitInfo {
id: string;
number: number;
commit: string;
Expand Down
6 changes: 2 additions & 4 deletions node-src/git/getParentCommits.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function createClient({
builds,
prs,
}: {
repository: Repository;
repository: Pick<Repository, 'commitMap'>;
builds: [string, string][];
prs?: [string, string][];
}) {
Expand Down Expand Up @@ -67,7 +67,7 @@ const options = {};
// This is built in from TypeScript 4.5
type Awaited<T> = T extends Promise<infer U> ? U : T;

interface Repository {
export interface Repository {
dirname: string;
runGit: ReturnType<typeof makeRunGit>;
commitMap: Awaited<ReturnType<typeof generateGitRepository>>;
Expand Down Expand Up @@ -334,8 +334,6 @@ describe('getParentCommits', () => {
await checkoutCommit('D', 'main', repository);
const client = createClient({
repository: {
dirname: undefined,
runGit: undefined,
commitMap: {
Z: { hash: 'b0ff6070903ff046f769f958830d2ebf989ff981', committedAt: 1234 },
},
Expand Down
2 changes: 1 addition & 1 deletion node-src/git/getParentCommits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ const MergeCommitsQuery = gql`
}
}
`;
interface MergeCommitsQueryResult {
export interface MergeCommitsQueryResult {
app: {
mergedPullRequests: [
{
Expand Down
29 changes: 20 additions & 9 deletions node-src/git/mocks/mockIndex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@

// create a mock set of responses to the queries we run as part of our git algorithm

import type { MergeCommitsQueryResult } from '../getParentCommits';
import { Repository } from '../getParentCommits.test';

interface Build {
branch: string;
commit: string;
Expand Down Expand Up @@ -55,15 +58,17 @@ const mocks = {
prs: PR[],
{ mergeInfoList }: { mergeInfoList: MergeInfo[] }
) => {
const mergedPrs = [];
const mergedPrs: MergeCommitsQueryResult['app']['mergedPullRequests'][0][] = [];
for (const mergeInfo of mergeInfoList) {
const pr = prs.find((p) => p.mergeCommitHash === mergeInfo.commit);
const prLastBuild = pr && lastBuildOnBranch(builds, pr.headBranch);
mergedPrs.push({
lastHeadBuild: prLastBuild && {
commit: prLastBuild.commit,
},
});
if (prLastBuild) {
mergedPrs.push({
lastHeadBuild: {
commit: prLastBuild.commit,
},
});
}
}

return {
Expand All @@ -84,13 +89,19 @@ const mocks = {
*
* @returns A mock Index service for testing.
*/
export default function createMockIndex({ commitMap }, buildDescriptions, prDescriptions = []) {
export default function createMockIndex(
{ commitMap }: Pick<Repository, 'commitMap'>,
buildDescriptions: [string, string][],
prDescriptions: [string, string][] = []
) {
const builds = buildDescriptions.map(([name, branch], index) => {
let hash, committedAt;
let hash: string;
let committedAt: number;

if (commitMap[name]) {
const commitInfo = commitMap[name];
hash = commitInfo.hash;
committedAt = Number.parseInt(commitInfo.committedAt, 10) * 1000;
committedAt = Math.floor(commitInfo.committedAt) * 1000;
} else {
// Allow for test cases with a commit that is no longer in the history
hash = name;
Expand Down
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
4 changes: 2 additions & 2 deletions node-src/io/getDNSResolveAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ export class DNSResolveAgent extends Agent {
...options,
lookup(
hostname: string,
_options: dns.LookupOneOptions,
callback: (err: NodeJS.ErrnoException, address: string, family: number) => void
_options: dns.LookupOptions,
callback: (err: NodeJS.ErrnoException | null, address: string, family: number) => void
) {
dns.resolve(hostname, (err, addresses) => callback(err, addresses?.[0], 4));
},
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
2 changes: 2 additions & 0 deletions node-src/lib/findChangedPackageFiles.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ const execGitCommand = vi.mocked(git.execGitCommand);
const mockFileContents = (packagesCommitsByFile) => {
execGitCommand.mockImplementation(async (input) => {
const regexResults = /show\s([^:]*):(.*)/g.exec(input);
if (!regexResults) return '';

const commit = regexResults[1];
const fileName = regexResults[2];

Expand Down
4 changes: 2 additions & 2 deletions node-src/lib/getDependentStoryFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ export async function getDependentStoryFiles(
// trace changed dependencies, so we bail just in case.
ctx.turboSnap.bailReason = {
changedPackageFiles: [
...ctx.git.changedFiles.filter((file) => isPackageManifestFile(file)),
...(ctx.git.changedFiles?.filter((file) => isPackageManifestFile(file)) || []),
...changedPackageLockFiles,
],
};
Expand All @@ -272,7 +272,7 @@ export async function getDependentStoryFiles(
// TODO: refactor this function
// eslint-disable-next-line complexity
function traceName(name: string, tracePath: string[] = []) {
if (ctx.turboSnap.bailReason || isCsfGlob(name)) return;
if (ctx.turboSnap?.bailReason || isCsfGlob(name)) return;
if (shouldBail(name)) return;
const { id } = modulesByName.get(name) || {};
const normalizedName = namesById.get(id);
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
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
9 changes: 7 additions & 2 deletions node-src/lib/log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ import { format } from 'util';

import { errorSerializer } from './logSerializers';

interface QueueMessage {
type: LogType;
messages: string[];
}

const { DISABLE_LOGGING, LOG_LEVEL = '' } = process.env;
const LOG_LEVELS = { silent: 0, error: 1, warn: 2, info: 3, debug: 4 };
const DEFAULT_LEVEL = 'info';
Expand Down Expand Up @@ -95,7 +100,7 @@ export const createLogger = () => {
const args = new Set(process.argv.slice(2));
let interactive = !args.has('--debug') && !args.has('--no-interactive');
let enqueue = false;
const queue = [];
const queue: QueueMessage[] = [];

const log =
(type: LogType, logFileOnly?: boolean) =>
Expand Down Expand Up @@ -140,7 +145,7 @@ export const createLogger = () => {
},
flush: () => {
while (queue.length > 0) {
const { type, messages } = queue.shift();
const { type, messages } = queue.shift() as QueueMessage;
console.log('');
console[type](...messages);
}
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
2 changes: 1 addition & 1 deletion node-src/lib/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export const transitionTo =
};

export const getDuration = (ctx: Context) => {
const now = Number.isInteger(ctx.now) ? ctx.now : Date.now();
const now = (Number.isInteger(ctx.now) ? ctx.now : Date.now()) as number;
const duration = Math.round((now - ctx.startedAt) / 1000);
const seconds = pluralize('second', Math.floor(duration % 60), true);
if (duration < 60) return seconds;
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
Loading

0 comments on commit 2892fba

Please sign in to comment.