Skip to content

Commit

Permalink
[EngSys] add a workaround to load ESM tests for Mocha (#28556)
Browse files Browse the repository at this point in the history
Currently most of our '*.js' tests and sources under dist-esm/ are
transformed by `esm` package (now outdated and archived) before being
feeded to Mocha because when a package.json has `"type":
"commonjs"` thus Mocha treats `.js` files as CommonJS.

This PR takes another approach by telling Mocha that the loaded modules
under dist-esm/ are ESM via a custom loader (`esm4mocha.mjs`).
This is done mainly in two parts:

1. During resolving, this loader checks the path of resolved modules and
if it includes `"dist-esm/"` we set the format of the resolved module to
`"module"`.
2. Also in ESM the full extension is required for relative modules in
import/export declarations, so a transformation is applied
to add `".js"` extension to relative imports/exports on the fly when
loading the module.

ServiceBus integration-test:node script has been updated to use this
approach. For now, the dev-tool command is unrolled and the old
workaround is replaced by the new one:

```diff
- -r ../../../common/tools/esm-workaround -r esm
+ --loader=../../../common/tools/esm4mocha.mjs
```

In the future, the dev-tool command would be updated to use the new
workaround for the majority of our packages.
  • Loading branch information
jeremymeng authored Feb 14, 2024
1 parent b0f24ec commit b82222b
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 3 deletions.
94 changes: 94 additions & 0 deletions common/tools/esm4mocha.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { readFile, stat, constants } from "node:fs/promises";
import { dirname, join } from "node:path";
import { fileURLToPath } from "url";
import { Project } from "./dev-tool/node_modules/ts-morph/dist/ts-morph.js";

// if modules are loaded from dist-esm/ treat them as ESM
export async function resolve(specifier, context, defaultResolve) {
consoleDir({ label: "resolving", specifier, context });
const resolved = await defaultResolve(specifier, context, defaultResolve);
consoleDir({ resolved });
if (resolved.url.includes("dist-esm/")) {
resolved.format = "module";
}
return resolved;
}

// if modules are loaded from dist-esm/ transform code to have ".js" extension
// for relative imports/exports as that is required by ESM.
export async function load(url, context, defaultLoad) {
consoleDir({ label: "loading...", url, context });
if (url.includes("dist-esm/")) {
const path = fileURLToPath(url);
const source = await readFile(path);
const transformed = await addJsExtensionToRelativeModules(source, path);
return {
format: context.format,
source: transformed,
shortCircuit: true,
};
}
const { source } = await defaultLoad(url, context, defaultLoad);
return { format: context.format, source, shortCircuit: true };
}

async function updateSpecifierValueIfRelative(declaration, base) {
const specifierValue = declaration.getModuleSpecifierValue();
consoleDir({ base, specifierValue });
if (specifierValue?.startsWith(".")) {
const sourcePath = join(base, specifierValue);
try {
const s = await stat(sourcePath, constants.R_OK);
consoleDir({ sourcePath });
if (s.isDirectory()) {
declaration.setModuleSpecifier(`${specifierValue}/index.js`);
} else {
declaration.setModuleSpecifier(`${specifierValue}.js`);
}
} catch (ex) {
consoleDir({ error: ex });
declaration.setModuleSpecifier(`${specifierValue}.js`);
}
consoleLog(` specifier updated to ${declaration.getModuleSpecifierValue()}`);
}
}

async function addJsExtensionToRelativeModules(source, path) {
const base = dirname(path);
const project = new Project({ useInMemoryFileSystem: true });
const text = source.toString("utf-8");
const f = project.createSourceFile("file.ts", text);
const imports = f.getImportDeclarations();
const exports = f.getExportDeclarations();
consoleDir({
l: "input",
path,
head: text.substring(0, 1200),
imports: imports.map((i) => i.getText()),
exports: exports.map((e) => e.getText()),
});
for (const i of imports) {
await updateSpecifierValueIfRelative(i, base);
}
for (const e of exports) {
await updateSpecifierValueIfRelative(e, base);
}

consoleDir({ l: "output", head: f.getFullText().substring(0, 1200) });
return f.getFullText();
}

function consoleLog(...args) {
if (process.env.DEBUG && process.env.DEBUG.startsWith("esm4mocha")) {
console.log(args);
}
}

function consoleDir(...args) {
if (process.env.DEBUG && process.env.DEBUG.startsWith("esm4mocha")) {
console.dir(args);
}
}
26 changes: 26 additions & 0 deletions eng/tools/dependency-testing/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,27 @@ async function usePackageTestTimeout(testPackageJson, packageJsonContents) {
}
}

async function adjustEsmWorkaround(testPackageJson, packageJsonContents) {
if (packageJsonContents.scripts["integration-test:node"]) {
if (packageJsonContents.devDependencies["esm"]) {
return;
}

testPackageJson.scripts["integration-test:node"] = testPackageJson.scripts[
"integration-test:node"
].replace("-r esm-workaround.js -r esm", "--loader=./esm4mocha.mjs");
}
}

async function updateEsm4MochaMjs(commonToolsPath, filePath) {
let fileContent = await packageUtils.readFile(filePath);
const writeContent = fileContent.replace(
"./dev-tool/node_modules/ts-morph/dist/ts-morph.js",
`${commonToolsPath}/dev-tool/node_modules/ts-morph/dist/ts-morph.js`
);
await packageUtils.writeFile(filePath, writeContent);
}

/**
* This inserts the package.json from the templates into the test folder.
* It computes the different versions of the dependencies/ dev-dep in this package.json
Expand Down Expand Up @@ -148,6 +169,7 @@ async function insertPackageJson(
}
}
}
await adjustEsmWorkaround(testPackageJson, packageJsonContents);
console.log(testPackageJson);
const testPackageJsonPath = path.join(testPath, "package.json");
await packageUtils.writePackageJson(testPackageJsonPath, testPackageJson);
Expand Down Expand Up @@ -265,6 +287,7 @@ async function copyRepoFile(repoRoot, relativePath, fileName, targetPackagePath,
const testPath = path.join(targetPackagePath, testFolder);
const sourcePath = path.join(repoRoot, relativePath, fileName);
const destPath = path.join(testPath, fileName);
console.log(`copying file from ${sourcePath} to ${destPath}`);
fs.copyFileSync(sourcePath, destPath);
}

Expand Down Expand Up @@ -403,6 +426,9 @@ async function main(argv) {
await replaceSourceReferences(targetPackagePath, targetPackage.packageName, testFolder);
await copyRepoFile(repoRoot, "common/tools", "mocha-multi-reporter.js", targetPackagePath, testFolder);
await copyRepoFile(repoRoot, "common/tools", "esm-workaround.js", targetPackagePath, testFolder);
await copyRepoFile(repoRoot, "common/tools", "esm4mocha.mjs", targetPackagePath, testFolder);
const commonToolsPath = path.resolve(path.join(repoRoot, "common/tools"));
await updateEsm4MochaMjs(commonToolsPath, path.join(targetPackagePath, testFolder, "esm4mocha.mjs"));
await updateRushConfig(repoRoot, targetPackage, testFolder);
outputTestPath(targetPackage.projectFolder, sourceDir, testFolder);
}
Expand Down
3 changes: 1 addition & 2 deletions sdk/servicebus/service-bus/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"extract-api": "tsc -p . && api-extractor run --local",
"format": "dev-tool run vendored prettier --write --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"samples/**/*.{ts,js}\" \"src/**/*.ts\" \"test/**/*.ts\" \"samples-dev/**/*.ts\" \"*.{js,json}\"",
"integration-test:browser": "karma start --single-run",
"integration-test:node": "dev-tool run test:node-js-input --no-test-proxy=true -- --timeout 600000 --full-trace \"dist-esm/test/internal/**/*.spec.js\" \"dist-esm/test/public/**/*.spec.js\"",
"integration-test:node": "c8 mocha --loader=../../../common/tools/esm4mocha.mjs -r source-map-support/register --reporter ../../../common/tools/mocha-multi-reporter.js --reporter-option output=test-results.xml --full-trace --timeout 600000 --full-trace --full-trace \"dist-esm/test/internal/**/*.spec.js\" \"dist-esm/test/public/**/*.spec.js\"",
"integration-test": "npm run integration-test:node && npm run integration-test:browser",
"lint:fix": "eslint package.json api-extractor.json README.md src test --ext .ts,.javascript,.js --fix --fix-type [problem,suggestion]",
"lint": "eslint package.json api-extractor.json README.md src test --ext .ts,.javascript,.js",
Expand Down Expand Up @@ -142,7 +142,6 @@
"debug": "^4.1.1",
"dotenv": "^16.0.0",
"eslint": "^8.0.0",
"esm": "^3.2.18",
"https-proxy-agent": "^7.0.0",
"karma": "^6.2.0",
"karma-chrome-launcher": "^3.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import chai from "chai";
const should = chai.should();
import fs from "fs";
import path from "path";
import { fileURLToPath } from "url";
import { packageJsonInfo } from "../../../src/util/constants";

// Since we currently hardcode package name and version in `constants.ts` file,
Expand All @@ -13,8 +14,9 @@ describe("Ensure package name and version are consistent in SDK and package.json
it("Ensure constants.ts file is consistent with package.json", () => {
// if you use ts-node then the folder structure is not as deep. There's only ever one of these files
// so we'll just check both spots and make it easy to swap back and forth between dist and ts-node.
const dir = path.dirname(fileURLToPath(import.meta.url));
const packageJsonFilePath = ["../../../../package.json", "../../../package.json"]
.map((pj) => path.join(__dirname, pj))
.map((pj) => path.join(dir, pj))
.filter((pj) => fs.existsSync(pj))[0];

const rawFileContents = fs.readFileSync(packageJsonFilePath, { encoding: "utf-8" });
Expand Down
2 changes: 2 additions & 0 deletions sdk/servicebus/service-bus/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
{
"extends": "../../../tsconfig.package",
"compilerOptions": {
"target": "ES2020",
"module": "ES2020",
"declarationDir": "./types",
"outDir": "./dist-esm",
"lib": ["dom", "ES2018.AsyncIterable"],
Expand Down

0 comments on commit b82222b

Please sign in to comment.