Think no-new-require should be a recommendation

[jimmywarting]

...why? refactoring cjs to esm gets harder.
it's a footgun for ppl writing this kind of syntax where they do new require('app-header') or stuff like
app = require('express')()

chaining stuff on the require() is bad.
it's better / easier if it looks more like ESM syntax and follows this format const { xyz } = require(x)
it's not recommended that you do things such as require(x).foo().bar

ppl complain that refactoring a project to esm is hard and takes long time. but having no-new-require as a recommendation would make it easier for ppl by spending less time and also making PR/reviews easier.

[voxpelli]

const { xyz } = require(x) also maps badly to ESM as it would need to be rewritten to:

import x from 'x'
const { xyz } = x

An import { xyz } from 'x' would require xyz to be exported individually in an ESM file, a CJS file behaves as having module.exports exported as the default and ESM imports don't destructure objects, they pick individual exports.

That said, I'm neither pro or con on this.

[jimmywarting]

Yes, but it ofc depends on how the module that you are trying to import how it's exporting things.

The statement const { xyz } = require(x) is somewhat similar to import { xyz } from 'x', which could make it worthwhile.

When the module is exported in a way similar to combining the grouping and export last rules, a code refactor becomes as simple as a one-line diff:

- module.exports = {
+ export = {
    x, 
    y,
    z
  }

With this export approach, refactoring and also reviewing the code then becomes very straightforward and easy, and import { xyz } from 'x' comes closest to using ESM syntax without any breaking changes by still being a cjs project. where the transition from cjs to esm are very high. Thus, combining these rules makes transitioning from CJS to ESM much easier to refactor at a later stage when you want to get on the train.

However, certain cases, like app = require('express')(), debug = require('debug')('webtorrent'), and stat = require('node:fs').stat, and even new require('xyz') are more troublesome to refactor and time-consuming. Therefore, the recommendation is to stick to only destructuring require statements, as it is the most sensible approach for handling them by treating them more or less similar to how ESM syntax closely resembles.

By following this more or less best practice then they won't feel as reluctant to wanna make that transition from cjs to esm.

[scagood]

First, I think I agree that no-new-require should be recommended as I don't think it should be used in node. It ultimatly leads to misunderstandings in the code.

---

Secondly, I think you're talking about a possible new rule, something like no-nested-require (I am terrible at names, and open to suggestions 😂)

Here is a quick astexplorer example:
https://astexplorer.net/#/gist/1e0ae8545cf92ad0b8e6f45822bed6ae/ffbfdafbfd4f6a33b7c664e9e996ffaaeaa76124

👨‍💻 Here is the long texty example:

The rule could be something like this:

export const meta = {
  type: "problem",
  hasSuggestions: true,
  fixable: false
};

export function create(context) {
  console.info(context);
  return {
    ':not(VariableDeclarator, ExpressionStatement) > CallExpression[callee.name="require"]'(node) {
      context.report({
        node: node,
        message: `Unexpected nested require`
      });
    }
  };
}

Given:

require('dotenv');

const app1 = require('express')();

const express = require('express');
const app2 = express();

const stat = require('node:fs').stat;

You get:

// Unexpected nested require (at 3:14)
   const app1 = require('express')();
// -------------^

// Unexpected nested require (at 8:14)
   const stat = require('node:fs').stat;
// -------------^

---

Edit [2022-10-26 12:13]

With fixer -- https://astexplorer.net/#/gist/1e0ae8545cf92ad0b8e6f45822bed6ae/e6365520bd2d4de5008c23c92d043d72f0bb6b1b