From 4ab165b70a76eecce94249fbcbd0017239a14002 Mon Sep 17 00:00:00 2001 From: Leonardo Venturini Date: Mon, 27 May 2024 08:46:23 -0400 Subject: [PATCH 1/5] fix declaration detection --- lib/import-export-visitor.js | 11 ++++++++++- node/version.js | 2 +- package.json | 3 +++ test/tla/exported-declaration.js | 5 +++++ test/tla/non-exported-declaration.js | 7 +++++++ test/top-level-await-tests.js | 12 +++++++++++- 6 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 test/tla/exported-declaration.js create mode 100644 test/tla/non-exported-declaration.js diff --git a/lib/import-export-visitor.js b/lib/import-export-visitor.js index 8e2a2e98..6622ced9 100644 --- a/lib/import-export-visitor.js +++ b/lib/import-export-visitor.js @@ -388,7 +388,16 @@ class ImportExportVisitor extends Visitor { visitAwaitExpression(path) { if (!this.hasTopLevelAwait) { - let parent = path.getParentNode(1); + const parent = path.getParentNode(1); + + if ( + parent.type === 'VariableDeclarator' && + (path.getParentNode(3).type === 'Program' || + path.getParentNode(4).type === 'Program') + ) { + this.hasTopLevelAwait = true; + } + if ( parent.type === 'ExpressionStatement' && path.getParentNode(2).type === 'Program' diff --git a/node/version.js b/node/version.js index 255187cd..f3dd3623 100644 --- a/node/version.js +++ b/node/version.js @@ -5,6 +5,6 @@ const path = require("path"); const pkgPath = path.join(__dirname, "../package.json"); const SemVer = require("semver"); -module.exports = new SemVer( +module.exports = SemVer.parse( process.env.REIFY_VERSION || fs.readJSON(pkgPath).version ); diff --git a/package.json b/package.json index bf92c263..eae5a026 100644 --- a/package.json +++ b/package.json @@ -44,5 +44,8 @@ "lodash": "4.17.21", "mocha": "9.1.1", "recast": "0.20.5" + }, + "volta": { + "node": "14.21.3" } } diff --git a/test/tla/exported-declaration.js b/test/tla/exported-declaration.js new file mode 100644 index 00000000..de734b63 --- /dev/null +++ b/test/tla/exported-declaration.js @@ -0,0 +1,5 @@ +export const test = await new Promise((resolve) => { + setTimeout(() => { + resolve('value'); + }, 1) +}) \ No newline at end of file diff --git a/test/tla/non-exported-declaration.js b/test/tla/non-exported-declaration.js new file mode 100644 index 00000000..8b6874e7 --- /dev/null +++ b/test/tla/non-exported-declaration.js @@ -0,0 +1,7 @@ +const test = await new Promise((resolve) => { + setTimeout(() => { + resolve('value'); + }, 1) +}) + +export const redirected = test \ No newline at end of file diff --git a/test/top-level-await-tests.js b/test/top-level-await-tests.js index 2d1aa4d8..558fcf32 100644 --- a/test/top-level-await-tests.js +++ b/test/top-level-await-tests.js @@ -3,7 +3,7 @@ const assert = require('assert'); const reify = require('../lib/runtime/index'); import { importSync, importAsync, importAsyncEvaluated } from './tla/nested/parent.js'; -(topLevelAwaitEnabled ? describe : describe.skip) ('top level await', () => { +(topLevelAwaitEnabled ? describe.only : describe.skip) ('top level await', () => { describe('evaluation order', () => { let logs = []; @@ -85,6 +85,16 @@ import { importSync, importAsync, importAsyncEvaluated } from './tla/nested/pare assert(exports.a === 1); }); + it('should detect exported declarations', async () => { + const exports = await require('./tla/exported-declaration.js'); + assert(exports.test === 'value'); + }) + + it('should detect non exported declarations', async () => { + const exports = await require('./tla/non-exported-declaration.js'); + assert(exports.redirected === 'value'); + }) + describe('errors', () => { it('should synchronously throw error for sync module', () => { try { From af9490c9dd80dddaef01dda18c839e10e408626f Mon Sep 17 00:00:00 2001 From: Leonardo Venturini Date: Mon, 27 May 2024 09:25:16 -0400 Subject: [PATCH 2/5] implement a more robust solution --- lib/import-export-visitor.js | 11 ++----- lib/is-top-level-await.js | 19 +++++++++++ test/tla/await-inside-function.js | 54 +++++++++++++++++++++++++++++++ test/top-level-await-tests.js | 9 ++++-- 4 files changed, 83 insertions(+), 10 deletions(-) create mode 100644 lib/is-top-level-await.js create mode 100644 test/tla/await-inside-function.js diff --git a/lib/import-export-visitor.js b/lib/import-export-visitor.js index 6622ced9..04589bcb 100644 --- a/lib/import-export-visitor.js +++ b/lib/import-export-visitor.js @@ -6,6 +6,7 @@ const utils = require("./utils.js"); const MagicString = require("magic-string"); const Visitor = require("./visitor.js"); +const { isTopLevelAwait } = require('./is-top-level-await'); const codeOfCR = "\r".charCodeAt(0); const codeOfDoubleQuote = '"'.charCodeAt(0); @@ -390,19 +391,13 @@ class ImportExportVisitor extends Visitor { if (!this.hasTopLevelAwait) { const parent = path.getParentNode(1); - if ( - parent.type === 'VariableDeclarator' && - (path.getParentNode(3).type === 'Program' || - path.getParentNode(4).type === 'Program') - ) { - this.hasTopLevelAwait = true; - } - if ( parent.type === 'ExpressionStatement' && path.getParentNode(2).type === 'Program' ) { this.hasTopLevelAwait = true; + } else { + this.hasTopLevelAwait = isTopLevelAwait(path.stack); } } diff --git a/lib/is-top-level-await.js b/lib/is-top-level-await.js new file mode 100644 index 00000000..f86b2f72 --- /dev/null +++ b/lib/is-top-level-await.js @@ -0,0 +1,19 @@ +const FUNCTION_TYPES = [ + 'FunctionDeclaration', + 'FunctionExpression', + 'ArrowFunctionExpression', + 'ClassMethod', + 'ObjectMethod', + 'ClassPrivateMethod', +] + +function isTopLevelAwait(ancestors) { + for (let ancestor of ancestors) { + if (FUNCTION_TYPES.includes(ancestor.type)) { + return false; + } + } + return true; +} + +module.exports = { isTopLevelAwait } \ No newline at end of file diff --git a/test/tla/await-inside-function.js b/test/tla/await-inside-function.js new file mode 100644 index 00000000..bac2bf48 --- /dev/null +++ b/test/tla/await-inside-function.js @@ -0,0 +1,54 @@ +// FunctionDeclaration +async function functionDeclaration() { + const result = await Promise.resolve("This is a function declaration"); + return result; +} + +// FunctionExpression +var functionExpression = async function() { + const result = await Promise.resolve("This is a function expression"); + return result; +}; + +// ArrowFunctionExpression +var arrowFunctionExpression = async () => { + const result = await Promise.resolve("This is an arrow function expression"); + return result; +}; + +// ClassMethod +class MyClass { + async classMethod() { + const result = await Promise.resolve("This is a class method"); + return result; + } +} + +// ObjectMethod +var myObject = { + objectMethod: async function() { + const result = await Promise.resolve("This is an object method"); + return result; + } +}; + +// ClassPrivateMethod +class MyClassWithPrivateMethod { + async #privateMethod() { + const result = await Promise.resolve("This is a private class method"); + return result; + } + + async callPrivateMethod() { + return this.#privateMethod(); + } +} + +module.exports = { + functionDeclaration, + functionExpression, + arrowFunctionExpression, + MyClass, + myObject, + MyClassWithPrivateMethod +}; \ No newline at end of file diff --git a/test/top-level-await-tests.js b/test/top-level-await-tests.js index 558fcf32..e5667662 100644 --- a/test/top-level-await-tests.js +++ b/test/top-level-await-tests.js @@ -85,16 +85,21 @@ import { importSync, importAsync, importAsyncEvaluated } from './tla/nested/pare assert(exports.a === 1); }); - it('should detect exported declarations', async () => { + it('should detect tla on exported declarations', async () => { const exports = await require('./tla/exported-declaration.js'); assert(exports.test === 'value'); }) - it('should detect non exported declarations', async () => { + it('should detect tla on non exported declarations', async () => { const exports = await require('./tla/non-exported-declaration.js'); assert(exports.redirected === 'value'); }) + it('should not detect tla if they are inside any function type', async () => { + const exports = require('./tla/await-inside-function.js'); + assert(!(exports instanceof Promise)) + }) + describe('errors', () => { it('should synchronously throw error for sync module', () => { try { From 6ea8f4c666c61f31d7eb3831aaf5226ff556bda9 Mon Sep 17 00:00:00 2001 From: Leonardo Venturini Date: Mon, 27 May 2024 09:26:58 -0400 Subject: [PATCH 3/5] adjust action nodejs versions --- .github/workflows/node.js.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml index bb4fede3..ff48b59f 100644 --- a/.github/workflows/node.js.yml +++ b/.github/workflows/node.js.yml @@ -16,7 +16,7 @@ jobs: strategy: matrix: - node-version: [10.x, 12.x, 14.x, 15.x, 16.x] + node-version: [14.x, 16.x, 18.x, 20.x, 22.x] # See supported Node.js release schedule at https://nodejs.org/en/about/releases/ steps: From 3bed2ec17758a2112cbebe2f2639e007e7f0d535 Mon Sep 17 00:00:00 2001 From: Leonardo Venturini Date: Mon, 27 May 2024 09:31:44 -0400 Subject: [PATCH 4/5] add test --- test/tla/exported-declaration-deep.js | 1 + test/top-level-await-tests.js | 5 +++++ 2 files changed, 6 insertions(+) create mode 100644 test/tla/exported-declaration-deep.js diff --git a/test/tla/exported-declaration-deep.js b/test/tla/exported-declaration-deep.js new file mode 100644 index 00000000..73a1f612 --- /dev/null +++ b/test/tla/exported-declaration-deep.js @@ -0,0 +1 @@ +export const value = Math.max(false ? 777 : await Promise.resolve(12), 2); \ No newline at end of file diff --git a/test/top-level-await-tests.js b/test/top-level-await-tests.js index e5667662..735c8ce6 100644 --- a/test/top-level-await-tests.js +++ b/test/top-level-await-tests.js @@ -95,6 +95,11 @@ import { importSync, importAsync, importAsyncEvaluated } from './tla/nested/pare assert(exports.redirected === 'value'); }) + it('should detect tla on exported declarations (deep)', async () => { + const exports = await require('./tla/exported-declaration-deep.js'); + assert(exports.value === 12); + }); + it('should not detect tla if they are inside any function type', async () => { const exports = require('./tla/await-inside-function.js'); assert(!(exports instanceof Promise)) From 0ff246a552bf55d62cbad45aa7d326e0e05ad9df Mon Sep 17 00:00:00 2001 From: Leonardo Venturini Date: Mon, 27 May 2024 09:39:27 -0400 Subject: [PATCH 5/5] oops --- test/top-level-await-tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/top-level-await-tests.js b/test/top-level-await-tests.js index 735c8ce6..00116ca2 100644 --- a/test/top-level-await-tests.js +++ b/test/top-level-await-tests.js @@ -3,7 +3,7 @@ const assert = require('assert'); const reify = require('../lib/runtime/index'); import { importSync, importAsync, importAsyncEvaluated } from './tla/nested/parent.js'; -(topLevelAwaitEnabled ? describe.only : describe.skip) ('top level await', () => { +(topLevelAwaitEnabled ? describe : describe.skip) ('top level await', () => { describe('evaluation order', () => { let logs = [];