From f132eebc12d0dcd71e5897503fd5d1a1ad526c50 Mon Sep 17 00:00:00 2001 From: Roman Kuznetsov Date: Fri, 3 May 2024 16:52:36 +0300 Subject: [PATCH] fix: dont remove imports in nodejs env --- src/bundle/test-transformer.ts | 33 +++++++++++++----------- src/test-reader/index.js | 6 ++++- src/test-reader/test-parser.js | 10 +++++-- src/test-reader/test-transformer.ts | 3 ++- src/worker/runner/simple-test-parser.js | 7 ++++- test/src/test-reader/test-parser.js | 15 +++++++++++ test/src/test-reader/test-transformer.ts | 28 +++++++++++++------- 7 files changed, 72 insertions(+), 30 deletions(-) diff --git a/src/bundle/test-transformer.ts b/src/bundle/test-transformer.ts index 07822de2b..2c5af56f5 100644 --- a/src/bundle/test-transformer.ts +++ b/src/bundle/test-transformer.ts @@ -6,7 +6,7 @@ import { TRANSFORM_EXTENSIONS, JS_EXTENSION_RE } from "./constants"; import type { NodePath, PluginObj, TransformOptions } from "@babel/core"; import type { ImportDeclaration } from "@babel/types"; -export const setupTransformHook = (): VoidFunction => { +export const setupTransformHook = (opts: { removeNonJsImports?: boolean } = {}): VoidFunction => { const transformOptions: TransformOptions = { browserslistConfigFile: false, babelrc: false, @@ -22,23 +22,26 @@ export const setupTransformHook = (): VoidFunction => { }, ], require("@babel/plugin-transform-modules-commonjs"), - [ - (): PluginObj => ({ - name: "ignore-imports", - visitor: { - ImportDeclaration(path: NodePath): void { - const extname = nodePath.extname(path.node.source.value); - - if (extname && !extname.match(JS_EXTENSION_RE)) { - path.remove(); - } - }, - }, - }), - ], ], }; + const customIgnoreImportsPlugin = (): PluginObj => ({ + name: "ignore-imports", + visitor: { + ImportDeclaration(path: NodePath): void { + const extname = nodePath.extname(path.node.source.value); + + if (extname && !extname.match(JS_EXTENSION_RE)) { + path.remove(); + } + }, + }, + }); + + if (opts.removeNonJsImports) { + transformOptions.plugins!.push([customIgnoreImportsPlugin]); + } + const revertTransformHook = addHook( (originalCode, filename) => { return babel.transform(originalCode, { filename, ...transformOptions })!.code as string; diff --git a/src/test-reader/index.js b/src/test-reader/index.js index 089d88963..464b0a7a4 100644 --- a/src/test-reader/index.js +++ b/src/test-reader/index.js @@ -30,7 +30,11 @@ module.exports = class TestReader extends EventEmitter { .useBrowsers(browsers) .build(process.cwd(), { ignore }, fileExtensions); - const parser = new TestParser(); + const testRunEnv = _.isArray(this.#config.system.testRunEnv) + ? this.#config.system.testRunEnv[0] + : this.#config.system.testRunEnv; + + const parser = new TestParser({ testRunEnv }); passthroughEvent(parser, this, [MasterEvents.BEFORE_FILE_READ, MasterEvents.AFTER_FILE_READ]); await parser.loadFiles(setCollection.getAllFiles(), this.#config); diff --git a/src/test-reader/test-parser.js b/src/test-reader/test-parser.js index 4b611e598..38d9f3a38 100644 --- a/src/test-reader/test-parser.js +++ b/src/test-reader/test-parser.js @@ -15,11 +15,17 @@ const clearRequire = require("clear-require"); const path = require("path"); class TestParser extends EventEmitter { + #opts; #buildInstructions; - constructor() { + /** + * @param {object} opts + * @param {"nodejs" | "browser" | undefined} opts.testRunEnv - environment to parse tests for + */ + constructor(opts = {}) { super(); + this.#opts = opts; this.#buildInstructions = new InstructionsList(); } @@ -51,7 +57,7 @@ class TestParser extends EventEmitter { this.#clearRequireCache(files); - const revertTransformHook = setupTransformHook(); + const revertTransformHook = setupTransformHook({ removeNonJsImports: this.#opts.testRunEnv === "browser" }); const rand = Math.random(); const esmDecorator = f => f + `?rand=${rand}`; diff --git a/src/test-reader/test-transformer.ts b/src/test-reader/test-transformer.ts index 980d27c04..0484b05f1 100644 --- a/src/test-reader/test-transformer.ts +++ b/src/test-reader/test-transformer.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/no-var-requires */ // TODO: use import instead require. Currently require is used because ts build filed from bundle folder even if it excluded in tsconfig -export const setupTransformHook: () => VoidFunction = require("../bundle").setupTransformHook; +export const setupTransformHook: (opts?: { removeNonJsImports?: boolean }) => VoidFunction = + require("../bundle").setupTransformHook; export const TRANSFORM_EXTENSIONS: string[] = require("../bundle").TRANSFORM_EXTENSIONS; diff --git a/src/worker/runner/simple-test-parser.js b/src/worker/runner/simple-test-parser.js index 69207db2f..7789133a0 100644 --- a/src/worker/runner/simple-test-parser.js +++ b/src/worker/runner/simple-test-parser.js @@ -1,5 +1,6 @@ "use strict"; +const _ = require("lodash"); const { EventEmitter } = require("events"); const { passthroughEvent } = require("../../events/utils"); const { TestParser } = require("../../test-reader/test-parser"); @@ -17,7 +18,11 @@ module.exports = class SimpleTestParser extends EventEmitter { } async parse({ file, browserId }) { - const parser = new TestParser(); + const testRunEnv = _.isArray(this._config.system.testRunEnv) + ? this._config.system.testRunEnv[0] + : this._config.system.testRunEnv; + + const parser = new TestParser({ testRunEnv }); passthroughEvent(parser, this, [WorkerEvents.BEFORE_FILE_READ, WorkerEvents.AFTER_FILE_READ]); diff --git a/test/src/test-reader/test-parser.js b/test/src/test-reader/test-parser.js index 29f63b1ee..341ce7107 100644 --- a/test/src/test-reader/test-parser.js +++ b/test/src/test-reader/test-parser.js @@ -374,6 +374,21 @@ describe("test-reader/test-parser", () => { }); describe("transform hook", () => { + describe("removeNonJsImports", () => { + [ + { removeNonJsImports: true, testRunEnv: "browser" }, + { removeNonJsImports: false, testRunEnv: "nodejs" }, + ].forEach(({ removeNonJsImports, testRunEnv }) => { + it(`should be "${removeNonJsImports}" if testRunEnv is "${testRunEnv}"`, async () => { + const parser = new TestParser({ testRunEnv }); + + await loadFiles_({ parser, files: ["foo/bar", "baz/qux"] }); + + assert.calledOnceWith(setupTransformHook, { removeNonJsImports }); + }); + }); + }); + it("should setup hook before read files", async () => { await loadFiles_({ files: ["foo/bar", "baz/qux"] }); diff --git a/test/src/test-reader/test-transformer.ts b/test/src/test-reader/test-transformer.ts index 0fb72d43d..ddc278eec 100644 --- a/test/src/test-reader/test-transformer.ts +++ b/test/src/test-reader/test-transformer.ts @@ -59,18 +59,26 @@ describe("test-transformer", () => { }); }); - describe("should not transform", () => { - [".css", ".less", ".scss", ".jpg", ".png", ".woff"].forEach(extName => { - it(`asset with extension: "${extName}"`, () => { - let transformedCode; - const fileName = `some${extName}`; - (pirates.addHook as SinonStub).callsFake(cb => { - transformedCode = cb(`import "${fileName}"`, fileName); - }); + [true, false].forEach(removeNonJsImports => { + describe(`should ${removeNonJsImports ? "" : "not "}remove non-js imports`, () => { + [".css", ".less", ".scss", ".jpg", ".png", ".woff"].forEach(extName => { + it(`asset with extension: "${extName}"`, () => { + let transformedCode; + const fileName = `some${extName}`; + (pirates.addHook as SinonStub).callsFake(cb => { + transformedCode = cb(`import "${fileName}"`, fileName); + }); - setupTransformHook(); + setupTransformHook({ removeNonJsImports }); + + const expectedCode = ['"use strict";']; - assert.equal(transformedCode, '"use strict";'); + if (!removeNonJsImports) { + expectedCode.push("", `require("some${extName}");`); + } + + assert.equal(transformedCode, expectedCode.join("\n")); + }); }); }); });