ronantakizawa/codereview-bench huggingface dataset:
Repository: webpack/webpack
File: lib/dependencies/HarmonyImportDependencyParserPlugin.js (JavaScript)

Code before:
			});
			if (dependencies.length > 0) {
				const dep = new HarmonyAcceptDependency(
					/** @type {Range} */
					(expr.range),
					dependencies,
					false
				);
				dep.loc = /** @type {DependencyLocation} */ (expr.loc);
				parser.state.module.addDependency(dep);
			}
		});
	}
};

/**
 * @param {JavascriptParser} parser parser
 * @param {ExportNamedDeclaration | ExportAllDeclaration | ImportDeclaration} node node
 * @param {boolean | undefined} deferImportEnabled defer import enabled
 * @param {boolean} reportSyntaxError report syntax error
 * @returns {{defer: boolean}} import attributes
 */
function getImportMode(parser, node, deferImportEnabled, reportSyntaxError) {
	const result = { defer: false };
	if ("phase" in node && node.phase === "defer") {
		if (deferImportEnabled) {
			result.defer = true;
		} else if (reportSyntaxError) {
			const error = new WebpackError(
				"Deferred import syntax (`import defer * as namespace from '...'`) cannot be used unless experimental.deferImport is true."
			);
			error.loc = node.loc || undefined;
			parser.state.current.addError(error);
		}
	}
	if (!node.range) {
		return result;
	}
	const { options, errors } = parser.parseCommentOptions(node.range);
	if (errors) {
		for (const e of errors) {
			const { comment } = e;
			if (!comment.loc) continue;
			parser.state.module.addWarning(
				new CommentCompilationWarning(
					`Compilation error while processing magic comment(-s): /*${comment.value}*/: ${e.message}`,
					comment.loc
				)
			);
		}
	}

Review comment (suggestion):
We don't need to check `deferImportEnabled` or `reportSyntaxError` here. 

The `node.phase` in AST is injected by `acorn-import-phases`, so it only exists when enabled `deferImport`. 

If disabled, one module parse error will occur instead. These module parse errors appear earliest.

Code after:
		});
	}
};

/**
 * @param {JavascriptParser} parser parser
 * @param {ExportNamedDeclaration | ExportAllDeclaration | ImportDeclaration} node node
 * @returns {{defer: boolean}} import attributes
 */
function getImportMode(parser, node) {
	const result = { defer: "phase" in node && node.phase === "defer" };
	if (!node.range) {
		return result;
	}
	const { options, errors } = parser.parseCommentOptions(node.range);
	if (errors) {
		for (const e of errors) {
			const { comment } = e;
			if (!comment.loc) continue;
			parser.state.module.addWarning(
				new CommentCompilationWarning(
					`Compilation error while processing magic comment(-s): /*${comment.value}*/: ${e.message}`,
					comment.loc
				)
			);
		}
	}
	if (!options) return result;
	if (options.webpackDefer) {
		if (typeof options.webpackDefer === "boolean") {
			result.defer = options.webpackDefer;
		} else if (node.loc) {
			parser.state.module.addWarning(
				new CommentCompilationWarning(
					"webpackDefer magic comment expected a boolean value.",
					node.loc
				)
			);
		}
	}
	return result;
}

module.exports.getImportMode = getImportMode;
module.exports.harmonySpecifierTag = harmonySpecifierTag;
Repository: webpack/webpack
File: lib/APIPlugin.js (JavaScript)

Code before:
			req: [RuntimeGlobals.initializeSharing],
			type: "function",
			assign: true
		}
	};
}

const PLUGIN_NAME = "APIPlugin";

const moduleCreateRequire = "__WEBPACK_EXTERNAL_createRequire";

class APIPlugin {
	/**
	 * Apply the plugin
	 * @param {Compiler} compiler the compiler instance
	 * @returns {void}
	 */
	apply(compiler) {
		compiler.hooks.compilation.tap(
			PLUGIN_NAME,
			(compilation, { normalModuleFactory }) => {
				const importMetaName = /** @type {string} */ (
					compilation.outputOptions.importMetaName
				);
				const moduleOutput = compilation.options.output.module;
				const nodeTarget = compilation.options.externalsPresets.node;
				const nodeEsm = moduleOutput && nodeTarget;

				const REPLACEMENTS = getReplacements();
				if (nodeEsm) {
					REPLACEMENTS.__non_webpack_require__.expr = `${moduleCreateRequire}(${importMetaName}.url)`;
				}

				compilation.dependencyTemplates.set(
					ConstDependency,
					new ConstDependency.Template()
				);

				compilation.hooks.runtimeRequirementInTree
					.for(RuntimeGlobals.chunkName)
					.tap(PLUGIN_NAME, (chunk) => {
						compilation.addRuntimeModule(
							chunk,
							new ChunkNameRuntimeModule(/** @type {string} */ (chunk.name))
						);
						return true;
					});

				compilation.hooks.runtimeRequirementInTree
					.for(RuntimeGlobals.getFullHash)
					.tap(PLUGIN_NAME, (chunk, _set) => {

Review comment (suggestion):
Better to use `compiler.platform.node` to check is it node target or not, don't forget we have `target: ['web', 'node']` (i.e. universal target, it is not fully ready right now, but we should finish it soon)

Code after:
			req: [RuntimeGlobals.initializeSharing],
			type: "function",
			assign: true
		}
	};
}

const PLUGIN_NAME = "APIPlugin";

const moduleCreateRequire = "__WEBPACK_EXTERNAL_createRequire";

class APIPlugin {
	/**
	 * Apply the plugin
	 * @param {Compiler} compiler the compiler instance
	 * @returns {void}
	 */
	apply(compiler) {
		compiler.hooks.compilation.tap(
			PLUGIN_NAME,
			(compilation, { normalModuleFactory }) => {
				const importMetaName = compilation.outputOptions.importMetaName;
				const moduleOutput = compilation.options.output.module;
				const nodeTarget = compiler.platform.node;
				const nodeEsm = moduleOutput && nodeTarget;

				const REPLACEMENTS = getReplacements();
				if (nodeEsm) {
					REPLACEMENTS.__non_webpack_require__.expr = `${moduleCreateRequire}(${importMetaName}.url)`;
				}

				compilation.dependencyTemplates.set(
					ConstDependency,
					new ConstDependency.Template()
				);

				compilation.hooks.runtimeRequirementInTree
					.for(RuntimeGlobals.chunkName)
					.tap(PLUGIN_NAME, (chunk) => {
						compilation.addRuntimeModule(
							chunk,
							new ChunkNameRuntimeModule(/** @type {string} */ (chunk.name))
						);
						return true;
					});

				compilation.hooks.runtimeRequirementInTree
					.for(RuntimeGlobals.getFullHash)
					.tap(PLUGIN_NAME, (chunk, _set) => {
						compilation.addRuntimeModule(chunk, new GetFullHashRuntimeModule());
						return true;
Repository: webpack/webpack
File: test/configCases/library/1-use-library/esm-with-bundled-commonjs.js (JavaScript)

Code before:
import lib1 from "lib1";
import lib2 from "lib2";
import lib3 from "lib3";
import lib4 from "lib4";
import lib5 from "lib5";


it(
	"should be able to import harmony exports from library (" + NAME + ")",
	function () {
		expect(new lib1().getNumber()).toBe(1);
		expect(lib2).toMatchObject({});
		expect(lib3.name).toBe("overrides-exports-cjs");
		expect(lib3.foo).toBe(undefined);
		expect(lib4).toMatchObject({});
		expect(lib5).toMatchObject({});
	}
);


Review comment (suggestion):
Is it expected, because I think we should have `adding-exports-cjs` and `foo`

Code after:
import lib1 from "lib1";
import lib2 from "lib2";
import lib3 from "lib3";
import lib4 from "lib4";
import lib5 from "lib5";


it(
	"should be able to import harmony exports from library (" + NAME + ")",
	function () {
		expect(new lib1().getNumber()).toBe(1);
		expect(lib2).toMatchObject({});
		expect(lib3.name).toBe("overrides-exports-cjs");
		expect(lib3.foo).toBe(undefined);
		expect(lib4).toEqual({});
		expect(lib5.name).toBe("adding-exports-cjs")
		expect(lib5.foo).toBe("foo")
	}
);
Repository: webpack/webpack
File: lib/ContextModule.js (JavaScript)

Code before:
							Template.toPath(dep.userRequest)
						);
					}
					const block = new AsyncDependenciesBlock(
						{
							...this.options.groupOptions,
							name: chunkName
						},
						dep.loc,
						dep.userRequest
					);
					block.addDependency(dep);
					this.addBlock(block);
				}
			} else {
				callback(
					new WebpackError(`Unsupported mode "${this.options.mode}" in context`)
				);
				return;
			}
			if (!this.context && !this.options.resource) return callback();

			const snapshotOptions = compilation.options.snapshot.contextModule;
			if (!snapshotOptions) {
				return callback();
			}

			compilation.fileSystemInfo.createSnapshot(
				startTime,
				null,
				this.context
					? [this.context]
					: typeof this.options.resource === "string"
						? [this.options.resource]
						: /** @type {string[]} */ (this.options.resource),
				null,
				snapshotOptions,
				(err, snapshot) => {
					if (err) return callback(err);
					/** @type {BuildInfo} */
					(this.buildInfo).snapshot = snapshot;
					callback();
				}
			);
		});
	}

	/**
	 * @param {LazySet<string>} fileDependencies set where file dependencies are added to
	 * @param {LazySet<string>} contextDependencies set where context dependencies are added to
	 * @param {LazySet<string>} missingDependencies set where missing dependencies are added to

Review comment (suggestion):
I think we don't need it because you can't set `contextModule: false` directly

Code after:
						chunkName = chunkName.replace(/\[index\]/g, `${index++}`);
						chunkName = chunkName.replace(
							/\[request\]/g,
							Template.toPath(dep.userRequest)
						);
					}
					const block = new AsyncDependenciesBlock(
						{
							...this.options.groupOptions,
							name: chunkName
						},
						dep.loc,
						dep.userRequest
					);
					block.addDependency(dep);
					this.addBlock(block);
				}
			} else {
				callback(
					new WebpackError(`Unsupported mode "${this.options.mode}" in context`)
				);
				return;
			}
			if (!this.context && !this.options.resource) return callback();

			const snapshotOptions = compilation.options.snapshot.contextModule;

			compilation.fileSystemInfo.createSnapshot(
				startTime,
				null,
				this.context
					? [this.context]
					: typeof this.options.resource === "string"
						? [this.options.resource]
						: /** @type {string[]} */ (this.options.resource),
				null,
				snapshotOptions,
				(err, snapshot) => {
					if (err) return callback(err);
					/** @type {BuildInfo} */
					(this.buildInfo).snapshot = snapshot;
					callback();
				}
			);
		});
	}

	/**
	 * @param {LazySet<string>} fileDependencies set where file dependencies are added to
	 * @param {LazySet<string>} contextDependencies set where context dependencies are added to
	 * @param {LazySet<string>} missingDependencies set where missing dependencies are added to
Repository: webpack/webpack
File: lib/asset/AssetModulesPlugin.js (JavaScript)

Code before:
const getAssetSourceGenerator = memoize(() =>
	require("./AssetSourceGenerator")
);

const type = ASSET_MODULE_TYPE;
const PLUGIN_NAME = "AssetModulesPlugin";

class AssetModulesPlugin {
	/**
	 * Apply the plugin
	 * @param {Compiler} compiler the compiler instance
	 * @returns {void}
	 */
	apply(compiler) {
		compiler.hooks.compilation.tap(
			PLUGIN_NAME,
			(compilation, { normalModuleFactory }) => {
				normalModuleFactory.hooks.createParser
					.for(ASSET_MODULE_TYPE)
					.tap(PLUGIN_NAME, (parserOptions) => {
						validateParserOptions(parserOptions);
						parserOptions = cleverMerge(
							/** @type {AssetParserOptions} */
							(compiler.options.module.parser.asset),
							parserOptions
						);

						let dataUrlCondition = parserOptions.dataUrlCondition;
						if (!dataUrlCondition || typeof dataUrlCondition === "object") {
							dataUrlCondition = {
								maxSize: 8096,
								...dataUrlCondition
							};
						}

						const AssetParser = getAssetParser();

						return new AssetParser(dataUrlCondition);
					});
				normalModuleFactory.hooks.createParser
					.for(ASSET_MODULE_TYPE_INLINE)
					.tap(PLUGIN_NAME, (_parserOptions) => {
						const AssetParser = getAssetParser();

						return new AssetParser(true);
					});
				normalModuleFactory.hooks.createParser
					.for(ASSET_MODULE_TYPE_RESOURCE)
					.tap(PLUGIN_NAME, (_parserOptions) => {
						const AssetParser = getAssetParser();


Review comment (suggestion):
We have `mergeGlobalOptions` in `NormalModuleFactory` where we merge global options with local options, so we don't need it here to make twice

Code after:
	require("./AssetSourceGenerator")
);

const type = ASSET_MODULE_TYPE;
const PLUGIN_NAME = "AssetModulesPlugin";

class AssetModulesPlugin {
	/**
	 * Apply the plugin
	 * @param {Compiler} compiler the compiler instance
	 * @returns {void}
	 */
	apply(compiler) {
		compiler.hooks.compilation.tap(
			PLUGIN_NAME,
			(compilation, { normalModuleFactory }) => {
				normalModuleFactory.hooks.createParser
					.for(ASSET_MODULE_TYPE)
					.tap(PLUGIN_NAME, (parserOptions) => {
						validateParserOptions(parserOptions);

						let dataUrlCondition = parserOptions.dataUrlCondition;
						if (!dataUrlCondition || typeof dataUrlCondition === "object") {
							dataUrlCondition = {
								maxSize: 8096,
								...dataUrlCondition
							};
						}

						const AssetParser = getAssetParser();

						return new AssetParser(dataUrlCondition);
					});
				normalModuleFactory.hooks.createParser
					.for(ASSET_MODULE_TYPE_INLINE)
					.tap(PLUGIN_NAME, (_parserOptions) => {
						const AssetParser = getAssetParser();

						return new AssetParser(true);
					});
				normalModuleFactory.hooks.createParser
					.for(ASSET_MODULE_TYPE_RESOURCE)
					.tap(PLUGIN_NAME, (_parserOptions) => {
						const AssetParser = getAssetParser();

						return new AssetParser(false);
					});
				normalModuleFactory.hooks.createParser
					.for(ASSET_MODULE_TYPE_SOURCE)
					.tap(PLUGIN_NAME, (_parserOptions) => {
						const AssetSourceParser = getAssetSourceParser();
Repository: webpack/webpack
File: test/BenchmarkTestCases.benchmark.mjs (JavaScript)

Code before:

		const finalizeAsyncRun = () => {
			finalizeBenchRun();
		};
		const finalizeSyncRun = () => {
			finalizeBenchRun();
		};

		bench.run = async () => {
			setupBenchRun();

			for (const task of bench.tasks) {
				const uri = getTaskUri(task.bench, task.name, rootCallingFile);
				await runTaskAsync(task, uri);
			}

			return finalizeAsyncRun();
		};

		bench.runSync = () => {
			setupBenchRun();

			for (const task of bench.tasks) {
				const uri = getTaskUri(task.bench, task.name, rootCallingFile);
				runTaskSync(task, uri);
			}

			return finalizeSyncRun();
		};
	} else if (codspeedRunnerMode === "walltime") {
		// TODO
	}

	return bench;
};

const bench = await withCodSpeed(
	new Bench({
		now: hrtimeNow,
		throws: true,
		warmup: true,
		warmupIterations: 2,
		iterations: 8,
		setup(task, mode) {
			console.log(`Setup (${mode} mode): ${task.name}`);
		},
		teardown(task, mode) {
			console.log(`Teardown (${mode} mode): ${task.name}`);
		}
	})
);

Review comment (performance):
We probably don’t need `walltime` mode here. We want to ignore system call latency and focus only on CPU instrumentation.

And the following is mentioned in `Codspeed` docs.

> If your the code you wish to optimize and measure relies heavily on system calls, you can use CodSpeed Macro Runners combined with our WallTime instrument. You can find more information in the [Walltime](https://codspeed.io/docs/instruments/walltime) section of the docs.



Code after:
		const finalizeSyncRun = () => {
			finalizeBenchRun();
		};

		bench.run = async () => {
			setupBenchRun();

			for (const task of bench.tasks) {
				const uri = getTaskUri(task.bench, task.name, rootCallingFile);
				await runTaskAsync(task, uri);
			}

			return finalizeAsyncRun();
		};

		bench.runSync = () => {
			setupBenchRun();

			for (const task of bench.tasks) {
				const uri = getTaskUri(task.bench, task.name, rootCallingFile);
				runTaskSync(task, uri);
			}

			return finalizeSyncRun();
		};
	} else if (codspeedRunnerMode === "walltime") {
		// We don't need it
	}

	return bench;
};

const bench = await withCodSpeed(
	new Bench({
		now: hrtimeNow,
		throws: true,
		warmup: true,
		warmupIterations: 2,
		iterations: 8,
		setup(task, mode) {
			console.log(`Setup (${mode} mode): ${task.name}`);
		},
		teardown(task, mode) {
			console.log(`Teardown (${mode} mode): ${task.name}`);
		}
	})
);

async function registerSuite(bench, test, baselines) {
	const testDirectory = path.join(casesPath, test);
	const optionsPath = path.resolve(testDirectory, "options.mjs");
Repository: webpack/webpack
File: lib/DotenvPlugin.js (JavaScript)

Code before:
		runningParsed[key] = resolvedValue;
	}

	// Part of `dotenv-expand` code, but we don't need it because of we don't modify `process.env`
	// for (const processKey in options.parsed) {
	// 	if (processEnv) {
	// 		processEnv[processKey] = options.parsed[processKey];
	// 	}
	// }

	return options;
}

/**
 * Format environment variables as DefinePlugin definitions
 * @param {Env} env environment variables
 * @param {string} importMetaName import.meta.name
 * @returns {Record<string, string>} formatted definitions
 */
const envToDefinitions = (env, importMetaName) => {
	const definitions = /** @type {Record<string, string>} */ ({});

	for (const [key, value] of Object.entries(env)) {
		const defValue = JSON.stringify(value);
		definitions[`process.env.${key}`] = defValue;
		definitions[`${importMetaName}.env.${key}`] = defValue;
	}

	return definitions;
};

class DotenvPlugin {
	/**
	 * @param {DotenvPluginOptions=} options options object
	 */
	constructor(options = {}) {
		validate(options);
		this.options = { ...DEFAULT_OPTIONS, ...options };
	}

	/**
	 * @param {Compiler} compiler the compiler instance
	 * @returns {void}
	 */
	apply(compiler) {
		const definePlugin = new compiler.webpack.DefinePlugin({});
		const prefixes = Array.isArray(this.options.prefix)
			? this.options.prefix
			: [this.options.prefix || "WEBPACK_"];
		/** @type {string | false} */
		const dir =

Review comment (suggestion):
We need to use `import.meta.env` directly here, `importMetaName` should be used only in output code, not in source, same `EnvironmentPlugin`

Code after:
		runningParsed[key] = resolvedValue;
	}

	// Part of `dotenv-expand` code, but we don't need it because of we don't modify `process.env`
	// for (const processKey in options.parsed) {
	// 	if (processEnv) {
	// 		processEnv[processKey] = options.parsed[processKey];
	// 	}
	// }

	return options;
}

/**
 * Format environment variables as DefinePlugin definitions
 * @param {Env} env environment variables
 * @returns {Record<string, string>} formatted definitions
 */
const envToDefinitions = (env) => {
	const definitions = /** @type {Record<string, string>} */ ({});

	for (const [key, value] of Object.entries(env)) {
		const defValue = JSON.stringify(value);
		definitions[`process.env.${key}`] = defValue;
		definitions[`import.meta.env.${key}`] = defValue;
	}

	return definitions;
};

class DotenvPlugin {
	/**
	 * @param {DotenvPluginOptions=} options options object
	 */
	constructor(options = {}) {
		validate(options);
		this.options = { ...DEFAULT_OPTIONS, ...options };
	}

	/**
	 * @param {Compiler} compiler the compiler instance
	 * @returns {void}
	 */
	apply(compiler) {
		const definePlugin = new compiler.webpack.DefinePlugin({});
		const prefixes = Array.isArray(this.options.prefix)
			? this.options.prefix
			: [this.options.prefix || "WEBPACK_"];
		/** @type {string | false} */
		const dir =
			typeof this.options.dir === "string"
Repository: webpack/webpack
File: lib/DotenvPlugin.js (JavaScript)

Code before:
	// }

	return options;
}

/**
 * Format environment variables as DefinePlugin definitions
 * @param {Record<string, string>} env environment variables
 * @returns {Record<string, string>} formatted definitions
 */
const envToDefinitions = (env) => {
	const definitions = /** @type {Record<string, string>} */ ({});

	for (const [key, value] of Object.entries(env)) {
		// Always use process.env. prefix for DefinePlugin
		definitions[`process.env.${key}`] = JSON.stringify(value);
	}

	return definitions;
};

class DotenvPlugin {
	/**
	 * @param {DotenvPluginOptions=} options options object
	 */
	constructor(options = {}) {
		validate(options);
		const prefix = Array.isArray(options.prefix)
			? options.prefix
			: [options.prefix || "WEBPACK_"];
		this.options = {
			...DEFAULT_OPTIONS,
			...options,
			prefix
		};
	}

	/**
	 * @param {Compiler} compiler the compiler instance
	 * @returns {void}
	 */
	apply(compiler) {
		const definePlugin = new compiler.webpack.DefinePlugin({});
		const fs = /** @type {InputFileSystem} */ (compiler.inputFileSystem);
		const fileSystemInfo = new FileSystemInfo(fs, {
			unmanagedPaths: compiler.unmanagedPaths,
			managedPaths: compiler.managedPaths,
			immutablePaths: compiler.immutablePaths,
			hashFunction: compiler.options.output.hashFunction
		});
		const cache = compiler.getCache(PLUGIN_NAME);

Review comment (suggestion):
We have `minLenght: 1`, in validate so it should be never happen

Code after:
	// 	if (processEnv) {
	// 		processEnv[processKey] = options.parsed[processKey];
	// 	}
	// }

	return options;
}

/**
 * Format environment variables as DefinePlugin definitions
 * @param {Env} env environment variables
 * @returns {Record<string, string>} formatted definitions
 */
const envToDefinitions = (env) => {
	const definitions = /** @type {Record<string, string>} */ ({});

	for (const [key, value] of Object.entries(env)) {
		// Always use process.env. prefix for DefinePlugin
		definitions[`process.env.${key}`] = JSON.stringify(value);
	}

	return definitions;
};

class DotenvPlugin {
	/**
	 * @param {DotenvPluginOptions=} options options object
	 */
	constructor(options = {}) {
		validate(options);
		this.options = { ...DEFAULT_OPTIONS, ...options };
	}

	/**
	 * @param {Compiler} compiler the compiler instance
	 * @returns {void}
	 */
	apply(compiler) {
		const definePlugin = new compiler.webpack.DefinePlugin({});
		const prefixes = Array.isArray(this.options.prefix)
			? this.options.prefix
			: [this.options.prefix || "WEBPACK_"];
		/** @type {string | false} */
		const dir =
			typeof this.options.dir === "string"
				? this.options.dir
				: typeof this.options.dir === "undefined"
					? compiler.context
					: this.options.dir;

		/** @type {undefined | Snapshot} */