Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix TLA not detecting top level declarations with await #8

Merged
merged 5 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/node.js.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 5 additions & 1 deletion lib/import-export-visitor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}

Expand Down
19 changes: 19 additions & 0 deletions lib/is-top-level-await.js
Original file line number Diff line number Diff line change
@@ -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 }
2 changes: 1 addition & 1 deletion node/version.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,8 @@
"lodash": "4.17.21",
"mocha": "9.1.1",
"recast": "0.20.5"
},
"volta": {
"node": "14.21.3"
}
}
54 changes: 54 additions & 0 deletions test/tla/await-inside-function.js
Original file line number Diff line number Diff line change
@@ -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
};
1 change: 1 addition & 0 deletions test/tla/exported-declaration-deep.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const value = Math.max(false ? 777 : await Promise.resolve(12), 2);
5 changes: 5 additions & 0 deletions test/tla/exported-declaration.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export const test = await new Promise((resolve) => {
setTimeout(() => {
resolve('value');
}, 1)
})
7 changes: 7 additions & 0 deletions test/tla/non-exported-declaration.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
const test = await new Promise((resolve) => {
setTimeout(() => {
resolve('value');
}, 1)
})

export const redirected = test
20 changes: 20 additions & 0 deletions test/top-level-await-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading