Skip to content

Commit

Permalink
fix: dont remove imports in nodejs env
Browse files Browse the repository at this point in the history
  • Loading branch information
KuznetsovRoman committed May 6, 2024
1 parent 728f6bb commit f132eeb
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 30 deletions.
33 changes: 18 additions & 15 deletions src/bundle/test-transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -22,23 +22,26 @@ export const setupTransformHook = (): VoidFunction => {
},
],
require("@babel/plugin-transform-modules-commonjs"),
[
(): PluginObj => ({
name: "ignore-imports",
visitor: {
ImportDeclaration(path: NodePath<ImportDeclaration>): 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<ImportDeclaration>): 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;
Expand Down
6 changes: 5 additions & 1 deletion src/test-reader/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 8 additions & 2 deletions src/test-reader/test-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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}`;
Expand Down
3 changes: 2 additions & 1 deletion src/test-reader/test-transformer.ts
Original file line number Diff line number Diff line change
@@ -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;
7 changes: 6 additions & 1 deletion src/worker/runner/simple-test-parser.js
Original file line number Diff line number Diff line change
@@ -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");
Expand All @@ -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]);

Expand Down
15 changes: 15 additions & 0 deletions test/src/test-reader/test-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"] });

Expand Down
28 changes: 18 additions & 10 deletions test/src/test-reader/test-transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
});
});
});
});
Expand Down

0 comments on commit f132eeb

Please sign in to comment.