diff --git a/.claude/unit-testing-jobs.md b/.claude/unit-testing-jobs.md new file mode 100644 index 000000000..f782f4ae1 --- /dev/null +++ b/.claude/unit-testing-jobs.md @@ -0,0 +1,157 @@ +# Unit Testing Job Code + +OpenFn job expressions are not valid JavaScript out of the box — top-level adaptor calls like `get('/endpoint')` prevent them from being imported directly into a test runner. Compiling them first solves this. + +## Approach + +1. Compile job expressions to standard JavaScript +2. Import compiled files in your test suite +3. Test any pure functions in isolation + +## Compiling for Tests + +`openfn compile` writes compiled output to `compiled/` by default. Use `--exports-only` to also strip adaptor operation calls, keeping only explicitly exported code. + +```bash +# Compile all workflows in the project (full compilation, preserves operations) +openfn compile + +# Compile all workflows, stripping operation calls (useful for unit testing) +openfn compile --exports-only + +# Compile a single workflow by name +openfn compile my-workflow + +# Compile a single workflow to a custom directory +openfn compile my-workflow -o tests/ + +# Compile a single job expression (prints to stdout) +openfn compile workflows/my-workflow/step-a.js --exports-only + +# Watch mode: recompile whenever source files change +openfn compile --exports-only --watch +``` + +## What `--exports-only` keeps + +`--exports-only` strips operation calls. Only explicitly exported declarations survive: + +- `export const myHelper = ...` ✓ +- `export function parseSms() {}` ✓ +- `const helper = ...` (not exported) ✗ — dropped +- `fn(state => ...)` (operation call) ✗ — stripped + +If an exported function depends on a non-exported local declaration, that dependency is kept automatically. + +`export default` is removed in strip mode — it is only needed by the runtime, not for unit testing. + +Steps with no exportable code (nothing is exported after stripping) are skipped — no file is written. + +## Full compilation (no `--exports-only`) + +Without `--exports-only`, the full compiled output is written — all declarations are preserved and operations are kept in `export default [op1, op2, ...]`: + +```js +import { post } from '@openfn/language-http'; + +export default [post('/endpoint', { data: state.data })]; +``` + +All steps are written regardless of whether they export anything. + +## Example: Testing a Helper Function + +**Source** (`workflows/dhis2-sync/transform.js`): + +```js +import { dateFns } from '@openfn/language-dhis2'; + +export const formatDate = (date) => dateFns.format(date, 'yyyy-MM-dd'); + +fn((state) => ({ + ...state, + data: state.data.map((row) => ({ + ...row, + date: formatDate(row.date), + })), +})); +``` + +**Compiled** (`compiled/dhis2-sync/transform.js`) after `openfn compile --exports-only`: + +```js +import { dateFns } from '@openfn/language-dhis2'; + +export const formatDate = (date) => dateFns.format(date, 'yyyy-MM-dd'); +``` + +The operation is stripped. `formatDate` survives because it is explicitly exported. + +**Test** (using any test runner): + +```js +// test/transform.test.js +import { formatDate } from '../compiled/dhis2-sync/transform.js'; + +test('formats a date correctly', () => { + const result = formatDate(new Date('2024-01-15')); + assert.equal(result, '2024-01-15'); +}); +``` + +## Project-wide Compilation + +Running `openfn compile` with no path compiles every step in every workflow in the current project directory (must contain `openfn.yaml`). + +Output layout: + +``` +compiled/ + my-workflow/ + step-a.js + step-b.js + another-workflow/ + step-c.js +``` + +Override the output directory with `-o `: + +```bash +openfn compile --exports-only -o tests/ +``` + +Configure default directories in `openfn.yaml`: + +```yaml +dirs: + workflows: workflows + compiled: compiled # used by openfn compile +``` + +### Stale file cleanup + +After each project-wide run with `--exports-only`, any step file that was skipped (no exportable code) is automatically deleted from `compiled/` if it exists from a previous run. Only files at exact step paths (`compiled//.js`) are touched — files you have added at other paths (e.g. `compiled/my-workflow/helpers.js`) are never removed. + +There is no option to wipe the entire `compiled/` directory. To do a full reset, delete it manually before running `openfn compile`. + +## Recommended Setup + +**`package.json`** (in your OpenFn project): + +```json +{ + "scripts": { + "compile": "openfn compile --exports-only", + "compile:watch": "openfn compile --exports-only --watch", + "test": "node --test test/**/*.test.js" + } +} +``` + +## Notes + +- In `--exports-only` mode, only `export const` and `export function` declarations survive — non-exported helpers are dropped unless referenced by an export +- Import statements are always preserved +- Without `--exports-only`, all code including operations is kept in `export default [...]` +- Watch mode reruns compilation on any source change, making the edit → test cycle fast +- Use `-O` to print compiled output to stdout instead of writing to disk diff --git a/claude.md b/claude.md index b2f45c047..c150056ca 100644 --- a/claude.md +++ b/claude.md @@ -111,6 +111,7 @@ The [.claude](.claude) folder contains detailed guides: - **[command-refactor.md](.claude/command-refactor.md)** - Refactoring CLI commands into project subcommand structure - **[event-processor.md](.claude/event-processor.md)** - Worker event processing architecture (batching, ordering) +- **[unit-testing-jobs.md](.claude/unit-testing-jobs.md)** - How to unit-test job code using `openfn compile --test` ## Code Standards diff --git a/packages/cli/package.json b/packages/cli/package.json index 78dca3c84..184dff1b8 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -60,6 +60,7 @@ "@openfn/project": "workspace:^", "@openfn/runtime": "workspace:*", "chalk": "^5.6.2", + "chokidar": "^3.6.0", "dotenv": "^17.3.1", "dotenv-expand": "^12.0.3", "figures": "^5.0.0", diff --git a/packages/cli/src/compile/command.ts b/packages/cli/src/compile/command.ts index b0f9029ac..d8b09552e 100644 --- a/packages/cli/src/compile/command.ts +++ b/packages/cli/src/compile/command.ts @@ -1,13 +1,14 @@ import yargs from 'yargs'; import { Opts } from '../options'; import * as o from '../options'; -import { build, ensure, override } from '../util/command-builders'; +import { build, ensure } from '../util/command-builders'; export type CompileOptions = Pick< Opts, | 'adaptors' | 'command' | 'expandAdaptors' + | 'exportsOnly' | 'ignoreImports' | 'expressionPath' | 'logJson' @@ -19,6 +20,8 @@ export type CompileOptions = Pick< | 'useAdaptorsMonorepo' | 'globals' | 'trace' + | 'watch' + | 'workflowName' > & { workflow?: Opts['workflow']; repoDir?: string; @@ -27,31 +30,30 @@ export type CompileOptions = Pick< const options = [ o.expandAdaptors, // order important o.adaptors, + o.exportsOnly, o.ignoreImports, o.inputPath, o.log, o.logJson, - override(o.outputStdout, { - default: true, - }), + o.outputStdout, o.outputPath, o.repoDir, o.trace, o.useAdaptorsMonorepo, + o.watchFlag, o.workflow, ]; const compileCommand: yargs.CommandModule = { command: 'compile [path]', describe: - 'Compile an openfn job or workflow and print or save the resulting JavaScript.', + 'Compile an openfn job, workflow, or whole project and print or save the resulting JavaScript.', handler: ensure('compile', options), builder: (yargs) => build(options, yargs) .positional('path', { describe: - 'The path to load the job or workflow from (a .js or .json file or a dir containing a job.js file)', - demandOption: true, + 'Path to a .js expression, .json/.yaml workflow, or a project directory. Omit to compile all workflows in the current project.', }) .example( 'compile foo/job.js', @@ -59,8 +61,33 @@ const compileCommand: yargs.CommandModule = { ) .example( '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' - ), + 'Compiles the workflow and writes to the given path' + ) + .example( + 'compile', + 'Compiles all workflows in the current project and writes JS files to compiled/' + ) + .example( + 'compile my-workflow', + 'Compiles a single workflow by name and writes JS files to compiled/' + ) + .example( + 'compile my-workflow -O', + 'Compiles a workflow and prints to stdout' + ) + .example( + 'compile foo/job.js --exports-only', + 'Strips adaptor operation calls, keeping only exported declarations' + ) + .example( + 'compile --exports-only', + 'Compiles entire project to compiled/ stripping operation calls' + ) + .example( + 'compile foo/job.js --watch', + 'Watches the file and recompiles on every change' + ) + .example('compile --watch', 'Compiles all workflows on change'), }; export default compileCommand; diff --git a/packages/cli/src/compile/compile.ts b/packages/cli/src/compile/compile.ts index 822188702..6271fa421 100644 --- a/packages/cli/src/compile/compile.ts +++ b/packages/cli/src/compile/compile.ts @@ -1,3 +1,5 @@ +import path from 'node:path'; +import fs from 'node:fs/promises'; import compile, { preloadAdaptorExports, Options, @@ -5,6 +7,7 @@ import compile, { } from '@openfn/compiler'; import { getModulePath, type ExecutionPlan, type Job } from '@openfn/runtime'; import type { SourceMapWithOperations } from '@openfn/lexicon'; +import { Workspace } from '@openfn/project'; import createLogger, { COMPILER, Logger } from '../util/logger'; import abort from '../util/abort'; @@ -12,6 +15,11 @@ import type { CompileOptions } from './command'; export type CompiledJob = { code: string; map?: SourceMapWithOperations }; +export const hasExportableCode = (code: string): boolean => + /^\s*(export\s+(const|let|var|function|class)|const|let|var|function|class)\s/m.test( + code + ); + export default async function ( job: ExecutionPlan, opts: CompileOptions, @@ -69,7 +77,6 @@ const compileJob = async ( } }; -// Find every expression in the job and run the compiler on it const compileWorkflow = async ( plan: ExecutionPlan, opts: CompileOptions, @@ -110,7 +117,6 @@ export const stripVersionSpecifier = (specifier: string) => { return specifier; }; -// Take a module path as provided by the CLI and convert it into a path export const resolveSpecifierPath = async ( pattern: string, repoDir: string | undefined, @@ -119,7 +125,6 @@ export const resolveSpecifierPath = async ( const [specifier, path] = pattern.split('='); if (path) { - // given an explicit path, just load it. log.debug(`Resolved ${specifier} to path: ${path}`); return path; } @@ -131,7 +136,6 @@ export const resolveSpecifierPath = async ( return null; }; -// Mutate the opts object to write export information for the add-imports transformer export const loadTransformOptions = async ( opts: CompileOptions, log: Logger @@ -140,15 +144,19 @@ export const loadTransformOptions = async ( logger: log || createLogger(COMPILER, opts as any), trace: opts.trace, }; - // If an adaptor is passed in, we need to look up its declared exports - // and pass them along to the compiler + + if (opts.exportsOnly) { + options['exports-only'] = true; + // ensure-exports and top-level-operations produce output incompatible with exports-only mode + options['ensure-exports'] = false; + options['top-level-operations'] = false; + } if (opts.adaptors?.length && opts.ignoreImports != true) { const adaptorsConfig = []; for (const adaptorInput of opts.adaptors) { let exports; const [specifier] = adaptorInput.split('='); - // Preload exports from a path, optionally logging errors in case of a failure log.debug(`Trying to preload types for ${specifier}`); const path = await resolveSpecifierPath(adaptorInput, opts.repoDir, log); if (path) { @@ -179,3 +187,105 @@ export const loadTransformOptions = async ( return options; }; + +export const compileProject = async ( + opts: CompileOptions, + log: Logger, + cwd = process.cwd(), + workflowFilter?: string +): Promise => { + // validate=false suppresses warnings when workspace config has no extra metadata + const workspace = new Workspace(cwd, log as any, false); + const project = await workspace.getCheckedOutProject(); + + if (!project) { + log.error( + 'No project found. Run from a directory containing openfn.yaml, or provide a path.' + ); + process.exit(1); + } + + const wsConfig = workspace.getConfig() as any; + + const compiledDir = opts.outputStdout + ? null + : path.resolve( + cwd, + opts.outputPath ?? wsConfig.dirs?.compiled ?? 'compiled' + ); + + if (compiledDir) { + log.info(`Compiling project to ${compiledDir}`); + } + + let workflows = project.workflows; + if (workflowFilter) { + workflows = workflows.filter( + (wf: any) => wf.id === workflowFilter || wf.name === workflowFilter + ); + if (workflows.length === 0) { + log.error(`Workflow '${workflowFilter}' not found in project.`); + process.exit(1); + } + } + + const outPaths: string[] = []; + const stalePaths: string[] = []; + + for (const workflow of workflows) { + for (const step of workflow.steps) { + const expression = (step as any).expression; + if (!expression || typeof expression !== 'string') continue; + + const adaptor: string | undefined = + (step as any).adaptor ?? (step as any).adaptors?.[0]; + const stepOpts: CompileOptions = { + ...opts, + adaptors: adaptor ? [adaptor] : opts.adaptors ?? [], + }; + + const { code } = await compileJob( + expression, + stepOpts, + log, + (step as any).name ?? step.id + ); + + if (opts.exportsOnly && !hasExportableCode(code)) { + const stalePath = compiledDir + ? path.join(compiledDir, workflow.id, `${step.id}.js`) + : null; + log.info( + ` ${workflow.id}/${step.id} — skipped (no exportable code after stripping)` + ); + if (stalePath) stalePaths.push(stalePath); + continue; + } + + if (opts.outputStdout) { + log.success(`// ${workflow.id}/${step.id}\n\n` + code); + } else { + const outPath = path.join(compiledDir!, workflow.id, `${step.id}.js`); + await fs.mkdir(path.dirname(outPath), { recursive: true }); + await fs.writeFile(outPath, code); + outPaths.push(outPath); + log.success(` ${workflow.id}/${step.id} → ${outPath}`); + } + } + } + + // Only deletes exact step paths — user-added files alongside them are untouched. + for (const stalePath of stalePaths) { + try { + await fs.unlink(stalePath); + log.info(` Removed stale ${stalePath}`); + } catch (e: any) { + if (e.code !== 'ENOENT') throw e; + } + } + + if (!opts.outputStdout) { + log.success(`Compiled ${outPaths.length} step(s) to ${compiledDir}`); + } + return outPaths; +}; diff --git a/packages/cli/src/compile/handler.ts b/packages/cli/src/compile/handler.ts index c92c62f80..fa2504464 100644 --- a/packages/cli/src/compile/handler.ts +++ b/packages/cli/src/compile/handler.ts @@ -1,15 +1,25 @@ -import { writeFile } from 'node:fs/promises'; +import path from 'node:path'; +import { writeFile, mkdir } from 'node:fs/promises'; +import chokidar from 'chokidar'; import type { CompileOptions } from './command'; import type { Logger } from '../util/logger'; -import compile from './compile'; +import compile, { compileProject } from './compile'; import loadPlan from '../util/load-plan'; import assertPath from '../util/assert-path'; -const compileHandler = async (options: CompileOptions, logger: Logger) => { - assertPath(options.path); +const collectWatchTargets = (options: CompileOptions): string[] => { + if (options.expressionPath) { + return [path.resolve(options.expressionPath)]; + } + if (options.path) { + return [path.resolve(options.path)]; + } + return [path.join(process.cwd(), 'workflows', '**', '*.js')]; +}; - let result; +const doCompile = async (options: CompileOptions, logger: Logger) => { + let result: string; if (options.expressionPath) { const { code } = await compile(options.expressionPath, options, logger); result = code; @@ -19,12 +29,57 @@ const compileHandler = async (options: CompileOptions, logger: Logger) => { result = JSON.stringify(compiledPlan, null, 2); } - if (options.outputStdout) { + if (options.outputPath) { + await mkdir(path.dirname(options.outputPath), { recursive: true }); + await writeFile(options.outputPath, result); + logger.success(`Compiled to ${options.outputPath}`); + } else { logger.success('Result:\n\n' + result); + } +}; + +const compileHandler = async (options: CompileOptions, logger: Logger) => { + if (options.workflowName) { + await compileProject(options, logger, process.cwd(), options.workflowName); + } else if (!options.path) { + await compileProject(options, logger); } else { - await writeFile(options.outputPath!, result as string); - logger.success(`Compiled to ${options.outputPath}`); + assertPath(options.path); + await doCompile(options, logger); } + + if (!options.watch) return; + + const watchTargets = collectWatchTargets(options); + logger.info(`Watching for changes. Ctrl+C to stop.`); + + const watcher = chokidar.watch(watchTargets, { + ignoreInitial: true, + ignored: ['**/node_modules/**', '**/compiled/**'], + }); + + watcher.on('change', async (changedPath: string) => { + logger.info(`${changedPath} changed, recompiling...`); + try { + if (options.workflowName) { + await compileProject( + options, + logger, + process.cwd(), + options.workflowName + ); + } else if (!options.path) { + await compileProject(options, logger); + } else { + await doCompile(options, logger); + } + } catch (e) { + logger.error('Compilation error:', e); + } + }); + + // Keep the process alive + await new Promise(() => {}); }; export default compileHandler; diff --git a/packages/cli/src/options.ts b/packages/cli/src/options.ts index a97df63b1..d56660ec8 100644 --- a/packages/cli/src/options.ts +++ b/packages/cli/src/options.ts @@ -66,8 +66,10 @@ export type Opts = { statePath?: string; stateStdin?: string; timeout?: number; // ms + exportsOnly?: boolean; trace?: boolean; useAdaptorsMonorepo?: boolean; + watch?: boolean; workflow: string; workflowName?: string; validate?: boolean; @@ -623,6 +625,26 @@ export const validate: CLIOption = { }, }; +export const exportsOnly: CLIOption = { + name: 'exports-only', + yargs: { + boolean: true, + description: + 'Strip adaptor operation calls, exporting only constants and functions', + default: false, + }, +}; + +export const watchFlag: CLIOption = { + name: 'watch', + yargs: { + alias: ['w'], + boolean: true, + description: 'Watch source files and recompile on change', + default: false, + }, +}; + export const workflow: CLIOption = { name: 'workflow', yargs: { diff --git a/packages/cli/test/compile/compile.test.ts b/packages/cli/test/compile/compile.test.ts index 1265fb44a..06c5eabce 100644 --- a/packages/cli/test/compile/compile.test.ts +++ b/packages/cli/test/compile/compile.test.ts @@ -7,6 +7,8 @@ import compile, { stripVersionSpecifier, loadTransformOptions, resolveSpecifierPath, + compileProject, + hasExportableCode, } from '../../src/compile/compile'; import { CompileOptions } from '../../src/compile/command'; import { mockFs, resetMockFs } from '../util'; @@ -344,3 +346,166 @@ test.serial('loadTransformOptions: ignore some imports', async (t) => { }); // TODO test exception if the module can't be found + +test.serial('loadTransformOptions: --exports-only enables exports-only transformer', async (t) => { + const opts = { exportsOnly: true } as CompileOptions; + const result = await loadTransformOptions(opts, mockLog); + t.is(result['exports-only'], true); + t.is(result['ensure-exports'], false); + t.is(result['top-level-operations'], false); +}); + +test.serial( + 'loadTransformOptions: --exports-only does not affect other options', + async (t) => { + const opts = { exportsOnly: true } as CompileOptions; + const result = await loadTransformOptions(opts, mockLog); + t.falsy(result['add-imports']); + } +); + +// hasExportableCode +test('hasExportableCode: returns true for const declaration', (t) => { + t.true(hasExportableCode("import x from 'y';\nconst helper = () => {};\nexport default [];")); +}); + +test('hasExportableCode: returns true for function declaration', (t) => { + t.true(hasExportableCode("function formatDate(d) { return d; }\nexport default [];")); +}); + +test('hasExportableCode: returns true for exported const', (t) => { + t.true(hasExportableCode("export const VALUE = 42;\nexport default [];")); +}); + +test('hasExportableCode: returns false when only imports', (t) => { + t.false(hasExportableCode("import { get } from '@openfn/language-http';")); +}); + +test('hasExportableCode: returns false for empty string', (t) => { + t.false(hasExportableCode('')); +}); + +test.serial( + 'compileProject: compiles all workflow steps and writes output files', + async (t) => { + const pnpm = path.resolve('../../node_modules/.pnpm'); + const recastPath = `${pnpm}/recast@0.21.5`; + const sourceMapPath = `${pnpm}/source-map@0.7.6`; + + mock({ + [recastPath]: mock.load(recastPath, {}), + [sourceMapPath]: mock.load(sourceMapPath, {}), + '/proj/openfn.yaml': ` +dirs: + workflows: /proj/workflows +`, + '/proj/workflows/wf1/wf1.yaml': ` +id: wf1 +steps: + - id: step-a + expression: "x();" +`, + }); + + const outPaths = await compileProject( + {} as CompileOptions, + mockLog, + '/proj' + ); + + t.is(outPaths.length, 1); + t.true(outPaths[0].endsWith('compiled/wf1/step-a.js')); + + const { default: nodeFsPromises } = await import('node:fs/promises'); + const code = await nodeFsPromises.readFile(outPaths[0], 'utf-8'); + t.true(code.includes('export default [x()]')); + + mock.restore(); + } +); + +test.serial( + 'compileProject: removes stale step files skipped in --exports-only run', + async (t) => { + const pnpm = path.resolve('../../node_modules/.pnpm'); + const recastPath = `${pnpm}/recast@0.21.5`; + const sourceMapPath = `${pnpm}/source-map@0.7.6`; + + // step-b has no exported code and will be skipped in --exports-only mode. + // Pre-populate a stale file at its expected output path. + mock({ + [recastPath]: mock.load(recastPath, {}), + [sourceMapPath]: mock.load(sourceMapPath, {}), + '/proj/openfn.yaml': ` +dirs: + workflows: /proj/workflows +`, + '/proj/workflows/wf1/wf1.yaml': ` +id: wf1 +steps: + - id: step-a + expression: "export const helper = () => {}; fn();" + - id: step-b + expression: "fn();" +`, + // stale file from a previous run without --exports-only + '/proj/compiled/wf1/step-b.js': 'export default [fn()];', + // user-added file — must not be touched + '/proj/compiled/wf1/step-b.test.js': 'import { } from "./step-b.js";', + }); + + await compileProject( + { exportsOnly: true } as CompileOptions, + mockLog, + '/proj' + ); + + const { default: nodeFsPromises } = await import('node:fs/promises'); + + // Stale step file should be gone + await t.throwsAsync(() => nodeFsPromises.readFile('/proj/compiled/wf1/step-b.js'), { + code: 'ENOENT', + }); + + // User file must still exist + const userFile = await nodeFsPromises.readFile('/proj/compiled/wf1/step-b.test.js', 'utf-8'); + t.truthy(userFile); + + mock.restore(); + } +); + +test.serial( + 'compileProject: respects outputPath as the compiled directory', + async (t) => { + const pnpm = path.resolve('../../node_modules/.pnpm'); + const recastPath = `${pnpm}/recast@0.21.5`; + const sourceMapPath = `${pnpm}/source-map@0.7.6`; + + mock({ + [recastPath]: mock.load(recastPath, {}), + [sourceMapPath]: mock.load(sourceMapPath, {}), + '/proj/openfn.yaml': ` +dirs: + workflows: /proj/workflows +`, + '/proj/workflows/wf1/wf1.yaml': ` +id: wf1 +steps: + - id: step-a + expression: "x();" +`, + }); + + const outPaths = await compileProject( + { outputPath: '/out' } as CompileOptions, + mockLog, + '/proj' + ); + + t.is(outPaths.length, 1); + t.true(outPaths[0].startsWith('/out/')); + + mock.restore(); + } +); diff --git a/packages/cli/test/compile/handler.test.ts b/packages/cli/test/compile/handler.test.ts new file mode 100644 index 000000000..f329a4e02 --- /dev/null +++ b/packages/cli/test/compile/handler.test.ts @@ -0,0 +1,7 @@ +import test from 'ava'; + +// hasExportableCode and compileProject tests live in compile.test.ts. +// This file is retained as a placeholder. +test('placeholder', (t) => { + t.pass(); +}); diff --git a/packages/cli/test/compile/options.test.ts b/packages/cli/test/compile/options.test.ts index a43b76b6e..720941a51 100644 --- a/packages/cli/test/compile/options.test.ts +++ b/packages/cli/test/compile/options.test.ts @@ -15,7 +15,7 @@ test('correct default options', (t) => { t.is(options.expandAdaptors, true); t.is(options.expressionPath, 'job.js'); t.falsy(options.logJson); // TODO this is undefined right now - t.is(options.outputStdout, true); + t.is(options.outputStdout, false); t.is(options.path, 'job.js'); t.falsy(options.useAdaptorsMonorepo); }); diff --git a/packages/compiler/src/transform.ts b/packages/compiler/src/transform.ts index 0e3245960..bac5e4f69 100644 --- a/packages/compiler/src/transform.ts +++ b/packages/compiler/src/transform.ts @@ -5,6 +5,7 @@ import createLogger, { Logger } from '@openfn/logger'; import addImports, { AddImportsOptions } from './transforms/add-imports'; import ensureExports from './transforms/ensure-exports'; +import exportsOnly from './transforms/exports-only'; import lazyState from './transforms/lazy-state'; import promises from './transforms/promises'; import topLevelOps, { @@ -15,6 +16,7 @@ import { heap } from './util'; export type TransformerName = | 'add-imports' | 'ensure-exports' + | 'exports-only' | 'top-level-operations' | 'test' | 'lazy-state'; @@ -38,6 +40,7 @@ export type TransformOptions = { ['add-imports']?: AddImportsOptions | boolean; ['ensure-exports']?: boolean; + ['exports-only']?: boolean; ['top-level-operations']?: TopLevelOpsOptions | boolean; ['test']?: any; ['lazy-state']?: any; @@ -60,6 +63,7 @@ export default function transform( if (!transformers) { transformers = [ + exportsOnly, lazyState, promises, ensureExports, diff --git a/packages/compiler/src/transforms/exports-only.ts b/packages/compiler/src/transforms/exports-only.ts new file mode 100644 index 000000000..3abb0105e --- /dev/null +++ b/packages/compiler/src/transforms/exports-only.ts @@ -0,0 +1,106 @@ +// Strips all non-exported top-level code (operations, export default, bare declarations). +// Runs before all other transformers (order: 0). No-op unless options === true. + +import { namedTypes as n } from 'ast-types'; +import type { NodePath } from 'ast-types/lib/node-path'; +import type { Transformer } from '../transform'; + +// Conservative: includes member expression property names to avoid false negatives. +export const collectRefs = ( + node: any, + refs = new Set() +): Set => { + if (!node || typeof node !== 'object') return refs; + if (Array.isArray(node)) { + node.forEach((item) => collectRefs(item, refs)); + return refs; + } + if (n.Identifier.check(node)) refs.add(node.name); + for (const key of Object.keys(node)) { + if ( + key === 'type' || + key === 'loc' || + key === 'comments' || + key === 'tokens' + ) + continue; + collectRefs(node[key], refs); + } + return refs; +}; + +export const buildDeclMap = ( + nodes: n.Statement[] +): Map => { + const map = new Map(); + for (const node of nodes) { + if (n.VariableDeclaration.check(node)) { + for (const d of node.declarations) { + if (n.Identifier.check((d as n.VariableDeclarator).id)) { + map.set(((d as n.VariableDeclarator).id as n.Identifier).name, node); + } + } + } else if (n.FunctionDeclaration.check(node) && node.id) { + map.set(node.id.name, node); + } + } + return map; +}; + +export const collectDeps = ( + seeds: n.Statement[], + declMap: Map +): Set => { + const result = new Set(seeds); + const queue = [...seeds]; + while (queue.length) { + const current = queue.shift()!; + for (const ref of Array.from(collectRefs(current))) { + const dep = declMap.get(ref); + if (dep && !result.has(dep)) { + result.add(dep); + queue.push(dep); + } + } + } + return result; +}; + +function visitor( + programPath: NodePath, + _logger: any, + options: boolean | {} = {} +) { + if (options !== true) return; + + const body = programPath.node.body; + + const nonExported = body.filter( + (node) => + !n.ImportDeclaration.check(node) && + !n.ExportNamedDeclaration.check(node) && + !n.ExportDefaultDeclaration.check(node) + ) as n.Statement[]; + + const declMap = buildDeclMap(nonExported); + const exportSeeds = body.filter((node) => + n.ExportNamedDeclaration.check(node) + ) as n.Statement[]; + const needed = collectDeps(exportSeeds, declMap); + + programPath.node.body = body.filter( + (node) => + n.ImportDeclaration.check(node) || + n.ExportNamedDeclaration.check(node) || + needed.has(node as n.Statement) + ) as any; + + return true; // abort further traversal +} + +export default { + id: 'exports-only', + types: ['Program'], + order: 0, + visitor, +} as unknown as Transformer; diff --git a/packages/compiler/src/transforms/top-level-operations.ts b/packages/compiler/src/transforms/top-level-operations.ts index a71263e5b..18f19bc54 100644 --- a/packages/compiler/src/transforms/top-level-operations.ts +++ b/packages/compiler/src/transforms/top-level-operations.ts @@ -5,8 +5,6 @@ import { namedTypes as n, namedTypes } from 'ast-types'; import type { NodePath } from 'ast-types/lib/node-path'; import type { Transformer } from '../transform'; -// Note that the validator should complain if it see anything other than export default [] -// What is the relationship between the validator and the compiler? export type ExtendedProgram = NodePath< namedTypes.Program & { @@ -16,10 +14,14 @@ export type ExtendedProgram = NodePath< export type TopLevelOpsOptions = { // Wrap operations in a `(state) => op` wrapper - wrap: boolean; // TODO + wrap?: boolean; // TODO }; -function visitor(programPath: ExtendedProgram) { +function visitor( + programPath: ExtendedProgram, + _logger: any, + _options: Partial = {} +) { const operations: Array<{ line: number; name: string; order: number }> = []; const children = programPath.node.body; const rem = []; @@ -44,12 +46,10 @@ function visitor(programPath: ExtendedProgram) { } programPath.node.body = rem; } else { - // error! there isn't an appropriate export statement - // What do we do? + // no export default [] — nothing to move operations into } programPath.node.operations = operations; - // if not (for now) we should cancel traversal return true; } diff --git a/packages/compiler/test/compile.test.ts b/packages/compiler/test/compile.test.ts index 614e26344..0f44148f5 100644 --- a/packages/compiler/test/compile.test.ts +++ b/packages/compiler/test/compile.test.ts @@ -278,3 +278,53 @@ test('respect ignore list when exports not provided', (t) => { const { code: result } = compile(source, options); t.is(result, expected); }); + +const exportsOnlyOpts = { + 'exports-only': true, + 'ensure-exports': false, + 'top-level-operations': false, +} as const; + +test('exports-only: removes top-level operations, keeps exported JS', (t) => { + const source = [ + 'export const formatDate = (d) => d.toISOString();', + 'get("/api");', + 'fn(state => ({ ...state, date: formatDate(state.data.date) }));', + ].join('\n'); + + const { code: result } = compile(source, exportsOnlyOpts); + + t.true(result.includes('export const formatDate')); + t.false(result.includes('get(')); + t.false(result.includes('fn(state')); + t.false(result.includes('export default []')); +}); + +test('exports-only: removes non-exported declarations', (t) => { + const source = [ + 'const formatDate = (d) => d.toISOString();', + 'get("/api");', + ].join('\n'); + + const { code: result } = compile(source, exportsOnlyOpts); + + // non-exported const is dropped; no export default [] + t.false(result.includes('const formatDate')); + t.false(result.includes('get(')); + t.false(result.includes('export default []')); +}); + +test('exports-only: keeps import statements', (t) => { + const source = [ + 'import { dateFns } from "@openfn/language-common";', + 'export const formatDate = (d) => dateFns.format(d);', + 'get("/api");', + 'export default [];', + ].join('\n'); + + const { code: result } = compile(source, exportsOnlyOpts); + + t.true(result.includes('import { dateFns }')); + t.true(result.includes('export const formatDate')); + t.false(result.includes('get(')); +}); diff --git a/packages/compiler/test/transforms/exports-only.test.ts b/packages/compiler/test/transforms/exports-only.test.ts new file mode 100644 index 000000000..a3ee658f4 --- /dev/null +++ b/packages/compiler/test/transforms/exports-only.test.ts @@ -0,0 +1,287 @@ +import test from 'ava'; +import { builders as b, namedTypes as n } from 'ast-types'; +import { print } from 'recast'; +import transform from '../../src/transform'; +import visitors, { + collectRefs, + buildDeclMap, + collectDeps, +} from '../../src/transforms/exports-only'; + +// Helpers +const makeConst = (name: string, value: any = b.literal(42)) => + b.variableDeclaration('const', [ + b.variableDeclarator(b.identifier(name), value), + ]); + +const makeExportConst = (name: string, value: any = b.literal(42)) => + b.exportNamedDeclaration(makeConst(name, value), []); + +const makeExportFn = ( + name: string, + body: any[] = [b.returnStatement(b.literal(1))] +) => + b.exportNamedDeclaration( + b.functionDeclaration(b.identifier(name), [], b.blockStatement(body)), + [] + ); + +const makeOp = (name: string) => + b.expressionStatement(b.callExpression(b.identifier(name), [])); + +const makeImport = (specifier: string, source: string) => + b.importDeclaration( + [b.importSpecifier(b.identifier(specifier))], + b.stringLiteral(source) + ); + +// --- collectRefs --- + +test('collectRefs: finds identifiers in a simple expression', (t) => { + const node = b.expressionStatement( + b.callExpression(b.identifier('fn'), [b.identifier('x')]) + ); + const refs = collectRefs(node); + t.true(refs.has('fn')); + t.true(refs.has('x')); +}); + +test('collectRefs: traverses nested nodes', (t) => { + const node = b.arrowFunctionExpression( + [], + b.callExpression(b.identifier('helper'), []) + ); + const refs = collectRefs(node); + t.true(refs.has('helper')); +}); + +// --- buildDeclMap --- + +test('buildDeclMap: maps variable declarations', (t) => { + const decl = makeConst('myVar'); + const map = buildDeclMap([decl]); + t.true(map.has('myVar')); + t.is(map.get('myVar'), decl); +}); + +test('buildDeclMap: maps function declarations', (t) => { + const decl = b.functionDeclaration( + b.identifier('myFn'), + [], + b.blockStatement([]) + ); + const map = buildDeclMap([decl]); + t.true(map.has('myFn')); +}); + +test('buildDeclMap: ignores export declarations', (t) => { + const decl = makeExportConst('exported'); + const map = buildDeclMap([decl]); + t.false(map.has('exported')); +}); + +// --- collectDeps --- + +test('collectDeps: includes seeds in result', (t) => { + const seed = makeConst('x'); + const result = collectDeps([seed], new Map()); + t.true(result.has(seed)); +}); + +test('collectDeps: transitively follows dependencies', (t) => { + const dep = makeConst('helper'); + const declMap = new Map([['helper', dep]]); + const seed = b.exportNamedDeclaration( + b.functionDeclaration( + b.identifier('doThing'), + [], + b.blockStatement([ + b.returnStatement(b.callExpression(b.identifier('helper'), [])), + ]) + ), + [] + ); + const result = collectDeps([seed], declMap); + t.true(result.has(dep)); +}); + +// --- exports-only transformer --- + +test('is a no-op when options is not true', (t) => { + const ast = b.program([ + makeOp('fn'), + b.exportDefaultDeclaration(b.arrayExpression([])), + ]); + const before = print(ast).code; + const after = print(transform(ast, [visitors])).code; + t.is(before, after); +}); + +test('is a no-op when options is false', (t) => { + const ast = b.program([makeOp('fn')]); + const before = print(ast).code; + const after = print( + transform(ast, [visitors], { 'exports-only': false }) + ).code; + t.is(before, after); +}); + +test('strips operation calls', (t) => { + const ast = b.program([makeOp('get'), makeOp('fn')]); + const { body } = transform(ast, [visitors], { + 'exports-only': true, + }) as n.Program; + t.is(body.length, 0); +}); + +test('strips export default []', (t) => { + const ast = b.program([ + makeOp('fn'), + b.exportDefaultDeclaration(b.arrayExpression([])), + ]); + const { body } = transform(ast, [visitors], { + 'exports-only': true, + }) as n.Program; + t.is(body.length, 0); +}); + +test('strips non-exported declarations', (t) => { + const ast = b.program([makeConst('x'), makeOp('fn')]); + const { body } = transform(ast, [visitors], { + 'exports-only': true, + }) as n.Program; + t.is(body.length, 0); +}); + +test('keeps import declarations', (t) => { + const imp = makeImport('get', '@openfn/language-http'); + const ast = b.program([imp, makeOp('fn')]); + const { body } = transform(ast, [visitors], { + 'exports-only': true, + }) as n.Program; + t.is(body.length, 1); + t.true(n.ImportDeclaration.check(body[0])); +}); + +test('keeps named export declarations', (t) => { + const exported = makeExportConst('helper'); + const ast = b.program([exported, makeOp('fn')]); + const { body } = transform(ast, [visitors], { + 'exports-only': true, + }) as n.Program; + t.is(body.length, 1); + t.true(n.ExportNamedDeclaration.check(body[0])); +}); + +test('keeps exported function declarations', (t) => { + const ast = b.program([makeExportFn('formatDate'), makeOp('fn')]); + const { body } = transform(ast, [visitors], { + 'exports-only': true, + }) as n.Program; + t.is(body.length, 1); + t.true(n.ExportNamedDeclaration.check(body[0])); +}); + +test('keeps non-exported declarations that an export depends on', (t) => { + const helper = makeConst( + 'fmt', + b.arrowFunctionExpression([b.identifier('d')], b.identifier('d')) + ); + const exported = makeExportFn('formatDate', [ + b.returnStatement( + b.callExpression(b.identifier('fmt'), [b.identifier('date')]) + ), + ]); + const ast = b.program([helper, exported, makeOp('fn')]); + const { body } = transform(ast, [visitors], { + 'exports-only': true, + }) as n.Program; + t.is(body.length, 2); + t.true(n.VariableDeclaration.check(body[0])); + t.true(n.ExportNamedDeclaration.check(body[1])); +}); + +test('drops non-exported declarations that no export depends on', (t) => { + const unused = makeConst('unused'); + const exported = makeExportFn('greet', [ + b.returnStatement(b.stringLiteral('hi')), + ]); + const ast = b.program([unused, exported, makeOp('fn')]); + const { body } = transform(ast, [visitors], { + 'exports-only': true, + }) as n.Program; + t.is(body.length, 1); + t.true(n.ExportNamedDeclaration.check(body[0])); +}); + +test('follows transitive dependencies', (t) => { + // exported → middle → leaf + const leaf = makeConst( + 'leaf', + b.arrowFunctionExpression([], b.stringLiteral('leaf')) + ); + const middle = makeConst( + 'middle', + b.callExpression(b.identifier('leaf'), []) + ); + const exported = makeExportFn('top', [ + b.returnStatement(b.callExpression(b.identifier('middle'), [])), + ]); + const ast = b.program([leaf, middle, exported, makeOp('fn')]); + const { body } = transform(ast, [visitors], { + 'exports-only': true, + }) as n.Program; + // leaf + middle + exported — no op, no export default + t.is(body.length, 3); +}); + +test('keeps imports alongside named exports', (t) => { + const imp = makeImport('dateFns', '@openfn/language-dhis2'); + const exported = makeExportConst('formatDate'); + const ast = b.program([imp, exported, makeOp('fn')]); + const { body } = transform(ast, [visitors], { + 'exports-only': true, + }) as n.Program; + t.is(body.length, 2); + t.true(n.ImportDeclaration.check(body[0])); + t.true(n.ExportNamedDeclaration.check(body[1])); +}); + +test('handles a file with only operations (no exports)', (t) => { + const ast = b.program([makeOp('fn'), makeOp('get')]); + const { body } = transform(ast, [visitors], { + 'exports-only': true, + }) as n.Program; + t.is(body.length, 0); +}); + +test('handles an empty file', (t) => { + const ast = b.program([]); + const { body } = transform(ast, [visitors], { + 'exports-only': true, + }) as n.Program; + t.is(body.length, 0); +}); + +test('handles multiple exports without operations', (t) => { + const ast = b.program([ + makeExportConst('a'), + makeExportConst('b'), + makeExportFn('c'), + ]); + const { body } = transform(ast, [visitors], { + 'exports-only': true, + }) as n.Program; + t.is(body.length, 3); +}); + +test('does not remove export default when exports-only is disabled', (t) => { + const ast = b.program([ + makeOp('fn'), + b.exportDefaultDeclaration(b.arrayExpression([])), + ]); + const { body } = transform(ast, [visitors], { + 'exports-only': false, + }) as n.Program; + t.is(body.length, 2); +}); diff --git a/packages/compiler/test/transforms/top-level-operations.test.ts b/packages/compiler/test/transforms/top-level-operations.test.ts index 3b069a394..f3a77020a 100644 --- a/packages/compiler/test/transforms/top-level-operations.test.ts +++ b/packages/compiler/test/transforms/top-level-operations.test.ts @@ -217,6 +217,7 @@ test('should only take the top of a nested operation call (and preserve its argu // TODO Does nothing if the export statement is wrong + test('appends an operations map to simple operation', (t) => { // We have to parse source here rather than building an AST so that we get positional information const { program } = parse(`fn(); export default [];`); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index fa9301662..c2e33ea96 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -191,6 +191,9 @@ importers: chalk: specifier: ^5.6.2 version: 5.6.2 + chokidar: + specifier: ^3.6.0 + version: 3.6.0 dotenv: specifier: ^17.3.1 version: 17.3.1