Revert "module.require" handling as "require" change

[andvgal]

Bug report

What is the current behavior?

Since v4.16.2, module.require is handled as require.

If the current behavior is a bug, please provide the steps to reproduce.

module.require has been used to hide import of non-browser modules in many projects.

There is a common idiom to add Node.js-specific functionality with various alternations:

const isNode = /* implementation defined */;

// Target-based implementation
const feature = isNode ? module.require( './node/feature' ) : require( './browser/feature' );

// Or extra features
const api = {};

if (isNode) {
    // adds or replaces some items
    module.require( './some/module' )( api );
}

Yes, the problem can be fixed in the project in question, but other software which already depends on older version of the project release gets still affected.

What is the expected behavior?

Ignore module.require as before v4.16.2.

Other relevant information:
webpack version: 4.16.5
Node.js version: N/A
Operating System: N/A
Additional tools: N/A

Related #7777 .

The change was introduced in v4.16.2 by PR #7750 for issue #7728 without any sane justification and risk analysis.

In any case, such serious behavior change should not be done without major webpack version change and/or without configuration option to control such behavior.

[sokra]

I did a code search on github and didn't found this pattern in the first 100 pages. So it doesn't seem so popular.

It was changed because it is considered as bugfix. Code like const x = module.require("x") reads with the intend to require x. This behavior is even documented in the node.js documentation and the CommonJS implementation of webpack is written with the intend to behave like node.js.

On the other side code like you mentioned above works because it relies on buggy behavior of webpack. It relies on module.require being broken.

We handle semver in a way that bugfixes are not considered as breaking changes by default, even if they are breaking changes when relying on buggy behavior. In this case fixing a bug is worth more than not breaking other code. There may be exceptions when many people are affected, but this doesn't seem to be the case here.

Note that there is a clear way of defining such "target-based" module replacements, which is the browser field:

const isNode = /* implementation defined */;

// Target-based implementation
const feature = require( './node/feature' );

// Or extra features
const api = {};

if (isNode) {
    // adds or replaces some items
    require( './some/module' )( api );
}

{
  "browser": {
    "./node/feature": "./browser/feature",
    "./some/module": false
  }
}

[andvgal]

OK, I am closing this for now.