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

lib, tools: remove duplicate requires #54987

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
5 changes: 1 addition & 4 deletions lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,13 @@ const {
} = primordials;
const kRejection = SymbolFor('nodejs.rejection');

const { SymbolDispose, kEmptyObject } = require('internal/util');
const { SymbolDispose, kEmptyObject, spliceOne } = require('internal/util');

const {
inspect,
identicalSequenceRange,
} = require('internal/util/inspect');

let spliceOne;
let FixedQueue;
let kFirstEventParam;
let kResistStopPropagation;
Expand Down Expand Up @@ -705,8 +704,6 @@ EventEmitter.prototype.removeListener =
if (position === 0)
list.shift();
else {
if (spliceOne === undefined)
spliceOne = require('internal/util').spliceOne;
spliceOne(list, position);
}

Expand Down
12 changes: 3 additions & 9 deletions lib/internal/abort_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ const {
kDeserialize,
kTransfer,
kTransferList,
markTransferMode,
} = require('internal/worker/js_transferable');

let _MessageChannel;
let markTransferMode;

const kDontThrowSymbol = Symbol('kDontThrowSymbol');

Expand All @@ -82,12 +82,6 @@ function lazyMessageChannel() {
return new _MessageChannel();
}

function lazyMarkTransferMode(obj, cloneable, transferable) {
markTransferMode ??=
require('internal/worker/js_transferable').markTransferMode;
markTransferMode(obj, cloneable, transferable);
}

const clearTimeoutRegistry = new SafeFinalizationRegistry(clearTimeout);
const gcPersistentSignals = new SafeSet();

Expand Down Expand Up @@ -168,7 +162,7 @@ class AbortSignal extends EventTarget {
this[kReason] = reason;
this[kComposite] = composite;
if (transferable) {
lazyMarkTransferMode(this, false, true);
markTransferMode(this, false, true);
}
}

Expand Down Expand Up @@ -425,7 +419,7 @@ class AbortController {
function transferableAbortSignal(signal) {
if (signal?.[kAborted] === undefined)
throw new ERR_INVALID_ARG_TYPE('signal', 'AbortSignal', signal);
lazyMarkTransferMode(signal, false, true);
markTransferMode(signal, false, true);
return signal;
}

Expand Down
4 changes: 1 addition & 3 deletions lib/internal/bootstrap/switches/does_own_process_state.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const {
parseFileMode,
validateArray,
validateString,
validateUint32,
} = require('internal/validators');

function wrapPosixCredentialSetters(credentials) {
Expand All @@ -42,9 +43,6 @@ function wrapPosixCredentialSetters(credentials) {
ERR_UNKNOWN_CREDENTIAL,
},
} = require('internal/errors');
const {
validateUint32,
} = require('internal/validators');

const {
initgroups: _initgroups,
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ const { setupMainThreadPort } = require('internal/worker/messaging');

const {
onGlobalUncaughtException,
evalScript,
evalModuleEntryPoint,
} = require('internal/process/execution');

let debug = require('internal/util/debuglog').debuglog('worker', (fn) => {
Expand Down Expand Up @@ -153,7 +155,6 @@ port.on('message', (message) => {
}

case 'classic': {
const { evalScript } = require('internal/process/execution');
const name = '[worker eval]';
// This is necessary for CJS module compilation.
// TODO: pass this with something really internal.
Expand All @@ -169,7 +170,6 @@ port.on('message', (message) => {
}

case 'module': {
const { evalModuleEntryPoint } = require('internal/process/execution');
PromisePrototypeThen(evalModuleEntryPoint(filename), undefined, (e) => {
workerOnGlobalUncaughtException(e, true);
});
Expand Down
4 changes: 1 addition & 3 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ const {
setHasStartedUserCJSExecution,
stripBOM,
toRealPath,
stripTypeScriptTypes,
} = require('internal/modules/helpers');
const packageJsonReader = require('internal/modules/package_json_reader');
const { getOptionValue, getEmbedderOptions } = require('internal/options');
Expand Down Expand Up @@ -1373,7 +1374,6 @@ function loadESMFromCJS(mod, filename) {
if (isUnderNodeModules(filename)) {
throw new ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING(filename);
}
const { stripTypeScriptTypes } = require('internal/modules/helpers');
source = stripTypeScriptTypes(source, filename);
}
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
Expand Down Expand Up @@ -1587,7 +1587,6 @@ function loadCTS(module, filename) {
throw new ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING(filename);
}
const source = getMaybeCachedSource(module, filename);
const { stripTypeScriptTypes } = require('internal/modules/helpers');
const code = stripTypeScriptTypes(source, filename);
module._compile(code, filename, 'commonjs');
}
Expand All @@ -1603,7 +1602,6 @@ function loadTS(module, filename) {
}
// If already analyzed the source, then it will be cached.
const source = getMaybeCachedSource(module, filename);
const { stripTypeScriptTypes } = require('internal/modules/helpers');
const content = stripTypeScriptTypes(source, filename);
let format;
const pkg = packageJsonReader.getNearestParentPackageJSON(filename);
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const { kEmptyObject } = require('internal/util');
const {
compileSourceTextModule,
getDefaultConditions,
forceDefaultLoader,
} = require('internal/modules/esm/utils');
const { kImplicitTypeAttribute } = require('internal/modules/esm/assert');
const { ModuleWrap, kEvaluating, kEvaluated } = internalBinding('module_wrap');
Expand Down Expand Up @@ -698,7 +699,7 @@ function createModuleLoader() {
// Don't spawn a new worker if custom loaders are disabled. For instance, if
// we're already in a worker thread created by instantiating
// CustomizedModuleLoader; doing so would cause an infinite loop.
if (!require('internal/modules/esm/utils').forceDefaultLoader()) {
if (!forceDefaultLoader()) {
const userLoaderPaths = getOptionValue('--experimental-loader');
if (userLoaderPaths.length > 0) {
if (!emittedLoaderFlagWarning) {
Expand Down
1 change: 0 additions & 1 deletion lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ function finalizeResolution(resolved, base, preserveSymlinks) {
try {
path = fileURLToPath(resolved);
} catch (err) {
const { setOwnProperty } = require('internal/util');
setOwnProperty(err, 'input', `${resolved}`);
setOwnProperty(err, 'module', `${base}`);
throw err;
Expand Down
1 change: 0 additions & 1 deletion lib/internal/modules/run_main.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ function resolveMainPath(main) {
} catch (err) {
if (defaultType === 'module' && err?.code === 'ENOENT') {
const { decorateErrorWithCommonJSHints } = require('internal/modules/esm/resolve');
const { getCWDURL } = require('internal/util');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review note: #54942

decorateErrorWithCommonJSHints(err, mainPath, getCWDURL());
}
throw err;
Expand Down
7 changes: 3 additions & 4 deletions lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const {
emitExperimentalWarning,
SymbolAsyncDispose,
SymbolDispose,
deprecate,
} = require('internal/util');

const {
Expand All @@ -43,6 +44,7 @@ const {
addSerializeCallback,
isBuildingSnapshot,
},
runDeserializeCallbacks,
} = require('internal/v8/startup_snapshot');

function prepareMainThreadExecution(expandArgv1 = false, initializeModules = true) {
Expand Down Expand Up @@ -141,7 +143,7 @@ function prepareExecution(options) {
initializeClusterIPC();

// TODO(joyeecheung): do this for worker threads as well.
require('internal/v8/startup_snapshot').runDeserializeCallbacks();
runDeserializeCallbacks();
} else {
assert(!internalBinding('worker').isMainThread);
// The setup should be called in LOAD_SCRIPT message handler.
Expand Down Expand Up @@ -464,7 +466,6 @@ function setupNetworkInspection() {
// this is used to deprecate APIs implemented in C++ where the deprecation
// utilities are not easily accessible.
function initializeDeprecations() {
const { deprecate } = require('internal/util');
const pendingDeprecation = getOptionValue('--pending-deprecation');

// DEP0103: access to `process.binding('util').isX` type checkers
Expand Down Expand Up @@ -526,8 +527,6 @@ function initializeDeprecations() {

function setupChildProcessIpcChannel() {
if (process.env.NODE_CHANNEL_FD) {
const assert = require('internal/assert');

const fd = NumberParseInt(process.env.NODE_CHANNEL_FD, 10);
assert(fd >= 0);

Expand Down
13 changes: 3 additions & 10 deletions lib/internal/process/warning.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ const { validateString } = require('internal/validators');
let fs;
let fd;
let warningFile;
let options;
let traceWarningHelperShown = false;

function resetForSerialization() {
Expand All @@ -42,15 +41,9 @@ function lazyOption() {
// This will load `warningFile` only once. If the flag is not set,
// `warningFile` will be set to an empty string.
if (warningFile === undefined) {
options = require('internal/options');
if (options.getOptionValue('--diagnostic-dir') !== '') {
warningFile = options.getOptionValue('--diagnostic-dir');
}
if (options.getOptionValue('--redirect-warnings') !== '') {
warningFile = options.getOptionValue('--redirect-warnings');
} else {
warningFile = '';
}
const diagnosticDir = getOptionValue('--diagnostic-dir');
const redirectWarnings = getOptionValue('--redirect-warnings');
warningFile = diagnosticDir || redirectWarnings || '';
}
return warningFile;
}
Expand Down
4 changes: 1 addition & 3 deletions lib/internal/source_map/source_map_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const {
setSourceMapsEnabled: setSourceMapsNative,
} = internalBinding('errors');
const {
defaultPrepareStackTrace,
setInternalPrepareStackTrace,
} = require('internal/errors');
const { getLazy } = require('internal/util');
Expand Down Expand Up @@ -64,9 +65,6 @@ function setSourceMapsEnabled(val) {
setInternalPrepareStackTrace(prepareStackTraceWithSourceMaps);
} else if (sourceMapsEnabled !== undefined) {
// Reset prepare stack trace callback only when disabling source maps.
const {
defaultPrepareStackTrace,
} = require('internal/errors');
setInternalPrepareStackTrace(defaultPrepareStackTrace);
}

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-eslint-duplicate-requires.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ new RuleTester({
}).run('no-duplicate-requires', rule, {
valid: [
{
code: 'require("a"); require("b"); (function() { require("a"); });',
code: '(function() { require("a"); }); (function() { require("a"); });',
},
{
code: 'require(a); require(a);',
Expand Down
61 changes: 24 additions & 37 deletions tools/eslint-rules/no-duplicate-requires.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* @fileoverview Ensure modules are not required twice at top level of a module
* @author devsnek
* @author RedYetiDev
*/
'use strict';

Expand All @@ -10,58 +11,44 @@ const { isRequireCall, isString } = require('./rules-utils.js');
// Rule Definition
//------------------------------------------------------------------------------

const secondLevelTypes = [
const topLevelTypes = new Set([
'FunctionDeclaration', 'FunctionExpression', 'ArrowFunctionExpression',
'ClassBody', 'MethodDefinition',
];
]);

function isTopLevel(node) {
while (!secondLevelTypes.includes(node.type)) {
const isTopLevel = (node) => {
while (node) {
if (topLevelTypes.has(node.type)) return false;
node = node.parent;
if (!node) {
return true;
}
}
return false;
}
return true;
};

module.exports = {
create(context) {
if (context.languageOptions.sourceType === 'module') {
return {};
}

function getRequiredModuleNameFromCall(node) {
// Node has arguments and first argument is string
if (node.arguments.length && isString(node.arguments[0])) {
return node.arguments[0].value.trim();
}

return undefined;
}

const required = new Set();

const rules = {
CallExpression: (node) => {
if (isRequireCall(node) && isTopLevel(node)) {
const moduleName = getRequiredModuleNameFromCall(node);
if (moduleName === undefined) {
return;
}
if (required.has(moduleName)) {
context.report(
node,
'\'{{moduleName}}\' require is duplicated.',
{ moduleName },
);
} else {
required.add(moduleName);
const requiredModules = new Set();

return {
CallExpression(node) {
if (isRequireCall(node)) {
const [firstArg] = node.arguments;
if (isString(firstArg)) {
const moduleName = firstArg.value.trim();
if (requiredModules.has(moduleName)) {
context.report({
node,
message: `'${moduleName}' require is duplicated.`,
});
} else if (isTopLevel(node)) {
requiredModules.add(moduleName);
}
}
}
},
};

return rules;
},
};
Loading