Skip to content

Commit

Permalink
Merge pull request #334 from OpenFn/fail-on-type-errors
Browse files Browse the repository at this point in the history
Fail on type errors
  • Loading branch information
josephjclark authored Jul 28, 2023
2 parents b3f85b1 + cb4d001 commit 0bb2672
Show file tree
Hide file tree
Showing 30 changed files with 140 additions and 112 deletions.
3 changes: 3 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ jobs:
- run:
name: Build packages
command: pnpm build
- run:
name: Type check
command: pnpm test:types
- run:
name: Run Tests
command: pnpm test
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"scripts": {
"test": "pnpm ava",
"test:watch": "pnpm ava -w",
"test:types": "pnpm tsc --noEmit --project tsconfig.json",
"test:types": "pnpm tsc --project tsconfig.test.json",
"build": "tsup --config ./tsup.config.js",
"build:watch": "pnpm build --watch",
"openfn": "node --no-warnings dist/index.js",
Expand Down Expand Up @@ -47,6 +47,7 @@
"typescript": "^4.7.4"
},
"dependencies": {
"@inquirer/prompts": "^1.1.4",
"@openfn/compiler": "workspace:*",
"@openfn/deploy": "workspace:*",
"@openfn/describe-package": "workspace:*",
Expand Down
31 changes: 18 additions & 13 deletions packages/cli/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,34 +15,39 @@ import testCommand from './test/command';
const y = yargs(hideBin(process.argv));

export const cmd = y
// TODO Typescipt hacks because signatures don't seem to align
.command(executeCommand as any)
.command(compileCommand as any)
.command(compileCommand)
.command(deployCommand as any)
.command(installCommand) // allow install to run from the top as well as repo
.command(repoCommand)
.command(testCommand)
.command(docsCommand)
.command(metadataCommand)
.command(docgenCommand)
.command(pullCommand)
.command(docgenCommand as any)
.command(pullCommand as any)
// Common options
.option('log', {
alias: ['l'],
description: 'Set the default log level to none, default, info or debug',
array: true,
})
.option('log-json', {
description: 'Output all logs as JSON objects',
boolean: true,
})
.example('openfn execute help', 'Show documentation for the execute command')
.example(
'openfn docs @openfn/language-common each',
'Get more help on the common.each command'
)
// TODO there's a bit of confusion right now about where log json lives
// Let's finish removing ensure-opts and sort this out there
// .option('log-json', {
// description: 'Output all logs as JSON objects',
// boolean: true,
// })
// .example('openfn execute help', 'Show documentation for the execute command')
// .example(
// 'openfn docs @openfn/language-common each',
// 'Get more help on the common.each command'
// )
.command({
command: 'version',
handler: (argv: Arguments<Opts>) => {
describe:
'Show the currently installed version of the CLI, compiler and runtime.',
handler: (argv: Arguments<Partial<Opts>>) => {
argv.command = 'version';
},
})
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const maybeEnsureOpts = (basePath: string, options: Opts) =>

// Top level command parser
const parse = async (basePath: string, options: Opts, log?: Logger) => {
const opts = maybeEnsureOpts(basePath, options);
const opts = maybeEnsureOpts(basePath, options) as SafeOpts;
const logger = log || createLogger(CLI, opts);

// In execute and test, always print version info FIRST
Expand Down
7 changes: 4 additions & 3 deletions packages/cli/src/compile/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ const options = [
o.useAdaptorsMonorepo,
];

const compileCommand = {
const compileCommand: yargs.CommandModule<CompileOptions> = {
command: 'compile [path]',
desc: 'Compile an openfn job or workflow and print or save the resulting JavaScript.',
describe:
'Compile an openfn job or workflow and print or save the resulting JavaScript.',
handler: ensure('compile', options),
builder: (yargs) =>
build(options, yargs)
Expand All @@ -56,6 +57,6 @@ const compileCommand = {
'compile foo/workflow.json -o foo/workflow-compiled.json',
'Compiles the workflow at foo/work.json and prints the result to -o foo/workflow-compiled.json'
),
} as yargs.CommandModule<CompileOptions>;
};

export default compileCommand;
3 changes: 2 additions & 1 deletion packages/cli/src/compile/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ export default async (opts: CompileOptions, log: Logger) => {
log.debug('Compiling...');
let job;
if (opts.workflow) {
job = compileWorkflow(opts.workflow, opts, log);
// Note that the workflow will be loaded into an object by this point
job = compileWorkflow(opts.workflow as ExecutionPlan, opts, log);
} else {
job = await compileJob((opts.job || opts.jobPath) as string, opts, log);
}
Expand Down
17 changes: 10 additions & 7 deletions packages/cli/src/deploy/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,22 @@ export type DeployOptions = Required<
| 'projectPath'
| 'configPath'
| 'confirm'
| 'describe'
>
>;

const options = [o.statePath, o.projectPath, o.configPath, o.confirm, o.describe];
const options = [
o.statePath,
o.projectPath,
o.configPath,
o.confirm,
o.describe,
];

const deployCommand = {
const deployCommand: yargs.CommandModule<DeployOptions> = {
command: 'deploy',
desc: "Deploy a project's config to a remote Lightning instance",
builder: (yargs: yargs.Argv<DeployOptions>) => {
return build(options, yargs).example('deploy', '');
},
describe: "Deploy a project's config to a remote Lightning instance",
handler: ensure('deploy', options),
builder: (yargs) => build(options, yargs),
};

export default deployCommand;
4 changes: 1 addition & 3 deletions packages/cli/src/deploy/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {
DeployError,
deploy,
getConfig,
getYaml,
validateConfig,
} from '@openfn/deploy';
import type { Logger } from '../util/logger';
Expand All @@ -27,7 +26,6 @@ async function deployHandler(
) {
try {
const config = mergeOverrides(await getConfig(options.configPath), options);


logger.debug('Deploying with config', JSON.stringify(config, null, 2));

Expand All @@ -36,7 +34,7 @@ async function deployHandler(
}

if (process.env['OPENFN_API_KEY']) {
logger.info('Using OPENFN_API_KEY environment variable');
logger.info('Using OPENFN_API_KEY environment variable');
config.apiKey = process.env['OPENFN_API_KEY'];
}

Expand Down
17 changes: 9 additions & 8 deletions packages/cli/src/docgen/command.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import yargs, { Arguments } from 'yargs';
import yargs, { ArgumentsCamelCase } from 'yargs';
import { Opts } from '../options';

const docgenCommand = {
type DocGenOptions = Partial<Opts>; // TODO

const docgenCommand: yargs.CommandModule<Opts> = {
command: 'docgen <specifier>',
// Hide this command as it's not really for public usage
desc: false, // 'Generate documentation into the repo. Specifier must include a version number.'
handler: (argv: Arguments<Opts>) => {
describe: false, // 'Generate documentation into the repo. Specifier must include a version number.'
handler: (argv: ArgumentsCamelCase<DocGenOptions>) => {
argv.command = 'docgen';
},
builder: (yargs: yargs.Argv) => {
return yargs.example('docgen @openfn/[email protected]', '');
},
} as yargs.CommandModule<Opts>;
builder: (yargs: yargs.Argv) =>
yargs.example('docgen @openfn/[email protected]', ''),
};

export default docgenCommand;
2 changes: 1 addition & 1 deletion packages/cli/src/docgen/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ const docgenHandler = (
// Return or wait for the existing docs
// If there's a timeout error, don't remove the placeholder
return waitForDocs(json, path, logger, retryDuration);
} catch (e) {
} catch (e: any) {
// Generate docs from scratch
if (e.message !== 'TIMEOUT') {
logger.info(`Docs JSON not found at ${path}`);
Expand Down
11 changes: 7 additions & 4 deletions packages/cli/src/docs/command.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import yargs, { Arguments } from 'yargs';
import yargs, { ArgumentsCamelCase } from 'yargs';
import { Opts } from '../options';

type DocsOptions = Partial<Opts>; // TODO

export default {
command: 'docs <adaptor> [operation]',
desc: 'Print help for an adaptor function. You can use short-hand for adaptor names (ie, common instead of @openfn/language-common)',
handler: (argv: Arguments<Opts>) => {
describe:
'Print help for an adaptor function. You can use short-hand for adaptor names (ie, common instead of @openfn/language-common)',
handler: (argv: ArgumentsCamelCase<DocsOptions>) => {
argv.command = 'docs';
},
builder: (yargs: yargs.Argv) =>
yargs.example('docs common fn', 'Print help for the common fn operation'),
} as unknown as yargs.CommandModule<Opts>;
} as yargs.CommandModule<DocsOptions>;
6 changes: 3 additions & 3 deletions packages/cli/src/execute/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ const options = [
o.useAdaptorsMonorepo,
];

const executeCommand = {
const executeCommand: yargs.CommandModule<ExecuteOptions> = {
command: 'execute [path]',
desc: `Run an openfn job or workflow. Get more help by running openfn <command> help.
describe: `Run an openfn job or workflow. Get more help by running openfn <command> help.
\nExecute will run a job/workflow at the path and write the output state to disk (to ./state.json unless otherwise specified)
\nBy default only state.data will be returned fron a job. Include --no-strict to write the entire state object.
\nRemember to include the adaptor name with -a. Auto install adaptors with the -i flag.`,
Expand Down Expand Up @@ -88,6 +88,6 @@ const executeCommand = {
'openfn compile job.js -a http',
'Compile job.js with the http adaptor and print the code to stdout'
),
} as yargs.CommandModule<ExecuteOptions>;
};

export default executeCommand;
4 changes: 2 additions & 2 deletions packages/cli/src/metadata/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ const options = [

export default {
command: 'metadata',
desc: 'Generate metadata for an adaptor config',
describe: 'Generate metadata for an adaptor config',
handler: ensure('metadata', options),
builder: (yargs) =>
build(options, yargs).example(
'metadata -a salesforce -s tmp/state.json',
'Generate salesforce metadata from config in state.json'
),
} as yargs.CommandModule<{}>;
} as yargs.CommandModule<MetadataOpts>;
2 changes: 1 addition & 1 deletion packages/cli/src/metadata/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Logger } from '../util/logger';
import { SafeOpts } from '../commands';
import loadState from '../util/load-state';
import cache from './cache';
import { getModuleEntryPoint, getNameAndVersion } from '@openfn/runtime';
import { getModuleEntryPoint } from '@openfn/runtime';

// Add extra, uh, metadata to the, uh, metadata object
const decorateMetadata = (metadata: any) => {
Expand Down
32 changes: 18 additions & 14 deletions packages/cli/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,17 @@ export type CLIOption = {
name: string;
// Allow this to take a function to lazy-evaluate env vars
yargs: yargs.Options | (() => yargs.Options);
ensure?: (opts: Opts) => void;
ensure?: (opts: Partial<Opts>) => void;
};

const setDefaultValue = (opts: Opts, key: keyof Opts, value: any) => {
const v: any = opts[key];
if (isNaN(v) && !v) {
const setDefaultValue = (
opts: Partial<Opts>,
key: keyof Opts,
value: unknown
) => {
const v = opts[key];
if (isNaN(v as number) && !v) {
// @ts-ignore
opts[key] = value;
}
};
Expand Down Expand Up @@ -134,21 +139,20 @@ export const configPath: CLIOption = {
alias: ['c', 'config-path'],
description: 'The location of your config file',
default: './.config.json',
}
},
};

export const describe: CLIOption = {
name: 'describe',
yargs : {
boolean: true,
description: "Downloads the project yaml from the specified instance"
},
ensure: (opts) => {
name: 'describe',
yargs: {
boolean: true,
description: 'Downloads the project yaml from the specified instance',
},
ensure: (opts) => {
setDefaultValue(opts, 'describe', true);
},
};


export const expandAdaptors: CLIOption = {
name: 'no-expand-adaptors',
yargs: {
Expand Down Expand Up @@ -194,7 +198,7 @@ export const ignoreImports: CLIOption = {
},
};

const getBaseDir = (opts: Opts) => {
const getBaseDir = (opts: { path?: string }) => {
const basePath = opts.path ?? '.';
if (/\.(jso?n?)$/.test(basePath)) {
return path.dirname(basePath);
Expand Down Expand Up @@ -274,7 +278,7 @@ export const projectPath: CLIOption = {
string: true,
alias: ['p'],
description: 'The location of your project.yaml file',
}
},
};

export const repoDir: CLIOption = {
Expand Down
17 changes: 8 additions & 9 deletions packages/cli/src/pull/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,21 @@ import * as o from '../options';
export type DeployOptions = Required<
Pick<
Opts,
| 'command'
| 'log'
| 'logJson'
| 'statePath'
| 'projectPath'
| 'configPath'
'command' | 'log' | 'logJson' | 'statePath' | 'projectPath' | 'configPath'
>
>;

const options = [o.statePath, o.projectPath, o.configPath];

const pullCommand = {
const pullCommand: yargs.CommandModule<DeployOptions> = {
command: 'pull',
desc: "Pull aproject's state and spec from a Lightning Instance to the local directory",
describe:
"Pull a project's state and spec from a Lightning Instance to the local directory",
builder: (yargs: yargs.Argv<DeployOptions>) => {
return build(options, yargs).example('pull', 'Pull an updated copy of a project spec and state from a Lightning Instance');
return build(options, yargs).example(
'pull',
'Pull an updated copy of a project spec and state from a Lightning Instance'
);
},
handler: ensure('pull', options),
};
Expand Down
Loading

0 comments on commit 0bb2672

Please sign in to comment.