From 45a76ec1b18062fcb9bf8534b3e9ff18c20d4b8f Mon Sep 17 00:00:00 2001 From: Farhan Yahaya Date: Wed, 8 Oct 2025 14:02:53 +0000 Subject: [PATCH 1/8] feat: ignore top objects in add-import --- .../compiler/src/transforms/add-imports.ts | 11 +++++++++++ .../test/transforms/add-imports.test.ts | 18 +++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/packages/compiler/src/transforms/add-imports.ts b/packages/compiler/src/transforms/add-imports.ts index 41dc24f42..aa3cd8716 100644 --- a/packages/compiler/src/transforms/add-imports.ts +++ b/packages/compiler/src/transforms/add-imports.ts @@ -107,6 +107,17 @@ export type IdentifierList = Record; export function findAllDanglingIdentifiers(ast: ASTNode) { const result: IdentifierList = {}; visit(ast, { + visitObjectExpression(path) { + // excaping top-level object variable declarations. + if ( + n.VariableDeclarator.check(path.parentPath.node) && + n.VariableDeclaration.check(path.parentPath.parentPath.node) && + n.Program.check(path.parentPath.parentPath.parentPath.parentPath.node) + ) { + return false; + } + this.traverse(path); + }, visitIdentifier: function (path) { // If this is part of an import statement, do nothing if (n.ImportSpecifier.check(path.parent.node)) { diff --git a/packages/compiler/test/transforms/add-imports.test.ts b/packages/compiler/test/transforms/add-imports.test.ts index de657ee7d..fda693122 100644 --- a/packages/compiler/test/transforms/add-imports.test.ts +++ b/packages/compiler/test/transforms/add-imports.test.ts @@ -147,7 +147,23 @@ test('findAllDanglingIdentifiers: const x = { a: 10 };', (t) => { t.assert(Object.keys(result).length == 0); }); -test('findAllDanglingIdentifiers: const x = { a: b };', (t) => { +test('findAllDanglingIdentifiers: ignore top-level objects declarations', (t) => { + // ignored because [bank] isn't picked as dangling but [foo] is + const ast = parse(` +const bigObj = {name: "ignore-me", age: bank}; +fn(state=> { + const update = { + name: "pick-me", + age: foo + } + return update +}) + `); + const result = findAllDanglingIdentifiers(ast); + t.deepEqual(result, { fn: true, foo: true }); +}); + +test.skip('findAllDanglingIdentifiers: const x = { a: b };', (t) => { const ast = parse('const x = { a: b };'); const result = findAllDanglingIdentifiers(ast); t.assert(Object.keys(result).length == 1); From 634783eac960b9da8fbb2181c75d1eaa8a3f4bfc Mon Sep 17 00:00:00 2001 From: "Farhan Y." Date: Wed, 8 Oct 2025 16:53:33 +0000 Subject: [PATCH 2/8] feat: ignore objects for lazy state (#1054) * feat: ignore objects for lazy state * tests: ignore top level object in lazy state --- packages/compiler/src/transforms/lazy-state.ts | 13 ++++++++++++- .../compiler/test/transforms/lazy-state.test.ts | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/compiler/src/transforms/lazy-state.ts b/packages/compiler/src/transforms/lazy-state.ts index dff6dfcb6..a41c5e841 100644 --- a/packages/compiler/src/transforms/lazy-state.ts +++ b/packages/compiler/src/transforms/lazy-state.ts @@ -79,6 +79,17 @@ const isOpenFunction = (path: NodePath) => { }; function visitor(path: NodePath) { + // if it was called for an ObjectExpression + if (n.ObjectExpression.check(path.node)) { + if ( + n.VariableDeclarator.check(path.parentPath.node) && + n.VariableDeclaration.check(path.parentPath.parentPath.node) && + n.Program.check(path.parentPath.parentPath.parentPath.parentPath.node) + ) { + return true; + } + return false; + } let first = path.node.object; while (first.hasOwnProperty('object')) { first = (first as n.MemberExpression).object; @@ -106,7 +117,7 @@ function visitor(path: NodePath) { export default { id: 'lazy-state', - types: ['MemberExpression'], + types: ['MemberExpression', 'ObjectExpression'], visitor, // It's important that $ symbols are escaped before any other transformations can run order: 0, diff --git a/packages/compiler/test/transforms/lazy-state.test.ts b/packages/compiler/test/transforms/lazy-state.test.ts index a941cdab1..55e96b449 100644 --- a/packages/compiler/test/transforms/lazy-state.test.ts +++ b/packages/compiler/test/transforms/lazy-state.test.ts @@ -15,6 +15,20 @@ test('convert a simple dollar reference', (t) => { t.is(code, 'get(state => state.data)'); }); +test.only("don't visit top-level object", (t) => { + const ast = parse(`const x = { x: $.data }; +fn($.data)`); + t.notThrows(() => { + const transformed = transform(ast, [visitors]); + const { code } = print(transformed); + t.is( + code, + `const x = { x: $.data }; +fn(state => state.data)` + ); + }); +}); + test('convert a chained dollar reference', (t) => { const ast = parse('get($.a.b.c.d)'); From e5f6b0efd4abc35562299ced72f4af8f3b1ae6dc Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Wed, 8 Oct 2025 18:44:04 +0100 Subject: [PATCH 3/8] remove .only --- packages/compiler/test/transforms/lazy-state.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/compiler/test/transforms/lazy-state.test.ts b/packages/compiler/test/transforms/lazy-state.test.ts index 55e96b449..9465de30c 100644 --- a/packages/compiler/test/transforms/lazy-state.test.ts +++ b/packages/compiler/test/transforms/lazy-state.test.ts @@ -15,7 +15,7 @@ test('convert a simple dollar reference', (t) => { t.is(code, 'get(state => state.data)'); }); -test.only("don't visit top-level object", (t) => { +test("don't visit top-level object", (t) => { const ast = parse(`const x = { x: $.data }; fn($.data)`); t.notThrows(() => { From 595a1f486627e5ec9b42fbd932ed0df405bbf54a Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Thu, 9 Oct 2025 09:38:58 +0100 Subject: [PATCH 4/8] better trace Use openfn compile --trace --- packages/compiler/src/transform.ts | 14 +++++++++++--- packages/compiler/src/util.ts | 4 ++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/compiler/src/transform.ts b/packages/compiler/src/transform.ts index 40bd06187..92d5b3d66 100644 --- a/packages/compiler/src/transform.ts +++ b/packages/compiler/src/transform.ts @@ -68,7 +68,6 @@ export default function transform( ] as Transformer[]; } const logger = options.logger || defaultLogger; - transformers // Ignore transformers which are explicitly disabled .filter(({ id }) => options[id] ?? true) @@ -82,11 +81,13 @@ export default function transform( }) // Run each transformer .forEach(({ id, types, visitor }) => { + const t = `transformer ${id}`; + console.time(t); const astTypes: Visitor = {}; for (const type of types) { const name = `visit${type}` as keyof Visitor; astTypes[name] = function (path: NodePath) { - printHeap(`visit: ${path.name}`); + // printHeap(`visit: ${path.name}`); const opts = options[id] || {}; const abort = visitor!(path, logger, opts); if (abort) { @@ -98,8 +99,15 @@ export default function transform( // @ts-ignore visit(ast, astTypes); + if (options.trace) { + console.log(); + printHeap(`${t}`); + console.timeEnd(t); + } }); - + if (options.trace) { + console.log(); + } printHeap(`finished`); const duration = (Date.now() - start) / 1000; logger.debug(`Finished in ${duration}s`); diff --git a/packages/compiler/src/util.ts b/packages/compiler/src/util.ts index 35107795f..f073b5b28 100644 --- a/packages/compiler/src/util.ts +++ b/packages/compiler/src/util.ts @@ -5,10 +5,10 @@ import path from 'node:path'; import { Project, describeDts } from '@openfn/describe-package'; import type { Logger } from '@openfn/logger'; -export function heap(reason: string, logger?: Logger) { +export function heap(reason: string, _logger?: Logger) { const { used_heap_size } = getHeapStatistics(); const mb = used_heap_size / 1024 / 1024; - logger?.debug(`[${reason}] Used heap at ${mb.toFixed(2)}mb`); + console.log(`[${reason}] Used heap at ${mb.toFixed(2)}mb`); } export const loadFile = (filePath: string) => From 2a199d75877e53fa25fcc043472d27d19e6f74a4 Mon Sep 17 00:00:00 2001 From: Farhan Yahaya Date: Thu, 9 Oct 2025 12:14:06 +0000 Subject: [PATCH 5/8] chore: optimise top level operations --- .../src/transforms/top-level-operations.ts | 82 ++++++++----------- .../transforms/top-level-operations.test.ts | 2 +- 2 files changed, 33 insertions(+), 51 deletions(-) diff --git a/packages/compiler/src/transforms/top-level-operations.ts b/packages/compiler/src/transforms/top-level-operations.ts index d4cbb8ae3..a71263e5b 100644 --- a/packages/compiler/src/transforms/top-level-operations.ts +++ b/packages/compiler/src/transforms/top-level-operations.ts @@ -8,71 +8,53 @@ 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 & { - operations: Array<{ line: number; name: string; order: number }>; -}; +export type ExtendedProgram = NodePath< + namedTypes.Program & { + operations: Array<{ line: number; name: string; order: number }>; + } +>; export type TopLevelOpsOptions = { // Wrap operations in a `(state) => op` wrapper wrap: boolean; // TODO }; -function visitor(path: NodePath) { - const root = path.parent.parent.node; - // Check if the node is a top level Operation +function visitor(programPath: ExtendedProgram) { + const operations: Array<{ line: number; name: string; order: number }> = []; + const children = programPath.node.body; + const rem = []; + + const target = programPath.node.body.at(-1); if ( - // Check this is a top level call expression - // ie, the parent must be an ExpressionStatement, and the statement's parent must be a Program - n.Program.check(root) && - n.Statement.check(path.parent.node) && - // An Operation could be an Identifier (fn()) or Member Expression (http.get()) - (n.Identifier.check(path.node.callee) || - n.MemberExpression.check(path.node.callee)) + n.ExportDefaultDeclaration.check(target) && + n.ArrayExpression.check(target.declaration) ) { - appendOperationMetadata(path, root as unknown as ExtendedProgram); - - // Now Find the top level exports array - const target = root.body.at(-1); - if ( - n.ExportDefaultDeclaration.check(target) && - n.ArrayExpression.check(target.declaration) - ) { - const statement = path.parent; - target.declaration.elements.push(path.node); - // orphan the original expression statement parent - statement.prune(); - } else { - // error! there isn't an appropriate export statement - // What do we do? + for (const child of children) { + if ( + n.ExpressionStatement.check(child) && + n.CallExpression.check(child.expression) + ) { + const order = operations.length + 1; + // @ts-ignore + const name = child.expression.callee.name; + const line = child.expression.loc?.start.line ?? -1; + operations.push({ name, line, order }); + target.declaration.elements.push(child.expression as any); + } else rem.push(child); } + programPath.node.body = rem; + } else { + // error! there isn't an appropriate export statement + // What do we do? } + programPath.node.operations = operations; // if not (for now) we should cancel traversal return true; } -// Add metadata to each operation for: -// - the order (do we know? We can guess from the state of the exports array -// - the name of the operation -// - the original line number (do we know?) -function appendOperationMetadata( - path: NodePath, - root: ExtendedProgram -) { - if (!root.operations) { - root.operations = []; - } - - const { operations } = root; - const order = operations.length + 1; - const name = path.value.callee.name; - const line = path.value.loc?.start.line ?? -1; - - operations.push({ line, order, name }); -} - export default { id: 'top-level-operations', - types: ['CallExpression'], + types: ['Program'], visitor, -} as Transformer; +} as unknown as Transformer; diff --git a/packages/compiler/test/transforms/top-level-operations.test.ts b/packages/compiler/test/transforms/top-level-operations.test.ts index 2361684ec..f9e8f49db 100644 --- a/packages/compiler/test/transforms/top-level-operations.test.ts +++ b/packages/compiler/test/transforms/top-level-operations.test.ts @@ -217,7 +217,7 @@ test('should only take the top of a nested operation call (and preserve its argu 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();`); + const { program } = parse(`fn(); export default [];`); transform(program, [visitors]); From 3ac2b151776f4cbec8063e456d351f266de291b0 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Thu, 9 Oct 2025 14:05:40 +0100 Subject: [PATCH 6/8] fix test fail --- packages/compiler/src/transform.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/compiler/src/transform.ts b/packages/compiler/src/transform.ts index 92d5b3d66..0e3245960 100644 --- a/packages/compiler/src/transform.ts +++ b/packages/compiler/src/transform.ts @@ -82,7 +82,7 @@ export default function transform( // Run each transformer .forEach(({ id, types, visitor }) => { const t = `transformer ${id}`; - console.time(t); + console.time?.(t); const astTypes: Visitor = {}; for (const type of types) { const name = `visit${type}` as keyof Visitor; @@ -102,7 +102,7 @@ export default function transform( if (options.trace) { console.log(); printHeap(`${t}`); - console.timeEnd(t); + console.timeEnd?.(t); } }); if (options.trace) { From cb97744ad7a6fef6a6a6d28384a41be0b93f6a80 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Thu, 9 Oct 2025 14:14:30 +0100 Subject: [PATCH 7/8] changeset --- .changeset/floppy-comics-work.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/floppy-comics-work.md diff --git a/.changeset/floppy-comics-work.md b/.changeset/floppy-comics-work.md new file mode 100644 index 000000000..e00ab5cc6 --- /dev/null +++ b/.changeset/floppy-comics-work.md @@ -0,0 +1,6 @@ +--- +'@openfn/compiler': patch +'@openfn/ws-worker': patch +--- + +Improve compiler performance for large files From 4c6c86d91de6a887152d9e02ac2b2c70aad3d60b Mon Sep 17 00:00:00 2001 From: Farhan Yahaya Date: Fri, 10 Oct 2025 15:40:00 +0000 Subject: [PATCH 8/8] feat: ignore object expression in several transforms --- packages/compiler/src/transform-ignore.ts | 20 +++++++++++++++++++ .../compiler/src/transforms/lazy-state.ts | 13 ++++-------- packages/compiler/src/transforms/promises.ts | 7 ++++++- 3 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 packages/compiler/src/transform-ignore.ts diff --git a/packages/compiler/src/transform-ignore.ts b/packages/compiler/src/transform-ignore.ts new file mode 100644 index 000000000..c798a1bc8 --- /dev/null +++ b/packages/compiler/src/transform-ignore.ts @@ -0,0 +1,20 @@ +import { namedTypes as n } from 'ast-types'; +import { NodePath } from 'ast-types/lib/node-path'; + +const IgnoreRules = (path: NodePath) => { + return { + check: n.ObjectExpression.check(path.node), + shouldSkip: () => { + if ( + n.VariableDeclarator.check(path.parentPath.node) && + n.VariableDeclaration.check(path.parentPath.parentPath.node) && + n.Program.check(path.parentPath.parentPath.parentPath.parentPath.node) + ) { + return true; + } + return false; + }, + }; +}; + +export default IgnoreRules; diff --git a/packages/compiler/src/transforms/lazy-state.ts b/packages/compiler/src/transforms/lazy-state.ts index a41c5e841..e1e578501 100644 --- a/packages/compiler/src/transforms/lazy-state.ts +++ b/packages/compiler/src/transforms/lazy-state.ts @@ -9,6 +9,7 @@ import { builders as b, namedTypes as n } from 'ast-types'; import type { NodePath } from 'ast-types/lib/node-path'; import type { Transformer } from '../transform'; +import IgnoreRules from '../transform-ignore'; // Walk up the AST and work out where the parent arrow function should go const ensureParentArrow = (path: NodePath) => { @@ -80,15 +81,9 @@ const isOpenFunction = (path: NodePath) => { function visitor(path: NodePath) { // if it was called for an ObjectExpression - if (n.ObjectExpression.check(path.node)) { - if ( - n.VariableDeclarator.check(path.parentPath.node) && - n.VariableDeclaration.check(path.parentPath.parentPath.node) && - n.Program.check(path.parentPath.parentPath.parentPath.parentPath.node) - ) { - return true; - } - return false; + const ignoreRule = IgnoreRules(path); + if (ignoreRule.check) { + return ignoreRule.shouldSkip(); } let first = path.node.object; while (first.hasOwnProperty('object')) { diff --git a/packages/compiler/src/transforms/promises.ts b/packages/compiler/src/transforms/promises.ts index 355c3e503..d189aec6c 100644 --- a/packages/compiler/src/transforms/promises.ts +++ b/packages/compiler/src/transforms/promises.ts @@ -1,6 +1,7 @@ import { namedTypes as n, builders as b } from 'ast-types'; import type { NodePath } from 'ast-types/lib/node-path'; +import IgnoreRules from '../transform-ignore'; const NO_DEFER_DECLARATION_ERROR = 'No defer declaration found'; @@ -146,6 +147,10 @@ const isTopScope = (path: NodePath) => { }; const visitor = (path: NodePath) => { + const ignoreRule = IgnoreRules(path); + if (ignoreRule.check) { + return ignoreRule.shouldSkip(); + } let root: NodePath = path; while (!n.Program.check(root.node)) { root = root.parent; @@ -166,7 +171,7 @@ const visitor = (path: NodePath) => { export default { id: 'promises', - types: ['CallExpression'], + types: ['CallExpression', 'ObjectExpression'], visitor, // this should run before top-level operations are moved into the exports array order: 0,