Merge pull request #8 from meteor/tla-assignment

Fix TLA not detecting top level declarations with await

.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:

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);
@@ -388,12 +389,15 @@ class ImportExportVisitor extends Visitor {
 
   visitAwaitExpression(path) {
     if (!this.hasTopLevelAwait) {
-      let parent = path.getParentNode(1);
+      const parent = path.getParentNode(1);
+
       if (
         parent.type === 'ExpressionStatement' &&
         path.getParentNode(2).type === 'Program'
       ) {
         this.hasTopLevelAwait = true;
+      } else {
+        this.hasTopLevelAwait = isTopLevelAwait(path.stack);
       }
     }
 

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 }

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
 );

package.json

@@ -44,5 +44,8 @@
     "lodash": "4.17.21",
     "mocha": "9.1.1",
     "recast": "0.20.5"
+  },
+  "volta": {
+    "node": "14.21.3"
   }
 }

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
+};

test/tla/exported-declaration-deep.js

@@ -0,0 +1 @@
+export const value = Math.max(false ? 777 : await Promise.resolve(12), 2);

test/tla/exported-declaration.js

@@ -0,0 +1,5 @@
+export const test = await new Promise((resolve) => {
+  setTimeout(() => {
+    resolve('value');
+  }, 1)
+})

test/tla/non-exported-declaration.js

@@ -0,0 +1,7 @@
+const test = await new Promise((resolve) => {
+  setTimeout(() => {
+    resolve('value');
+  }, 1)
+})
+
+export const redirected = test

test/top-level-await-tests.js

@@ -85,6 +85,26 @@ import { importSync, importAsync, importAsyncEvaluated } from './tla/nested/pare
     assert(exports.a === 1);
   });
 
+  it('should detect tla on exported declarations', async () => {
+    const exports = await require('./tla/exported-declaration.js');
+    assert(exports.test === 'value');
+  })
+
+  it('should detect tla on non exported declarations', async () => {
+    const exports = await require('./tla/non-exported-declaration.js');
+    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))
+  })
+
   describe('errors', () => {
     it('should synchronously throw error for sync module', () => {
       try {