Rule proposal: no-dynamic-require

[jfmengels]

I have just found require calls of the following form in a codebase

const type = '...';
const foo = require('../../bar/' + type + '/foo');

It can be useful when doing real dynamic package imports (with a plugin system for instance), but in most cases, it's not a good practice, for all the reasons that pushed the ES6 import/export system to be static.

I'm proposing a rule to forbid dynamic paths in require(), i.e. using something else than a string literal as a require argument. For the name, I think no-dynamic-require fits well.

I'm wondering whether template literals without expressions should be allowed.

Invalid

// Any non-static argument to require
var foo = require('../' + f + '/foo');
var foo = require(`../${f}/foo`);
var foo = require(f());
var foo = require(f + g);

Valid

var foo = require('../bar/foo');
import foo from '../bar/foo';

var foo = require(`../bar/foo`); // Allow this?

[benmosher]

👍 to allowing empty template literals iff it's easy to distinguish. I've never looked at the AST representation of them.

I'd expect 99.999% of them to have expressions, though, so if summarily rejecting template literals is a lot cleaner, I'd go with that until someone presents a compelling argument for them.

[jfmengels]

Yeah, it's pretty easy: https://astexplorer.net/#/ne6VWpNvM5
The TemplateLiteral nodes have an expressions array field, which is empty when there are no expressions.

I'd expect 99.999% of them to have expressions

Same here, unless for the people who use template literals as their default string (why not).

Anyway, let's include template literals as it's easy to check.

[gkatsanos]

We're using a config object in in order to use a different module in case of testing.

const config = require('../../config');

const Location = require(config.get('modules:app/models/location'));

how would you handle that case?

[ljharb]

I wouldn't do that to mock requires; that's changing production code for testing purposes. I'd use a static require and use one of the many available module mocking solutions in tests.

[gkatsanos]

thanks. It's a legacy project and I guess for now I'll add a todo/exception