Replace module.require in scope hoisting

[mischnic]

↪️ Pull Request

When scope hoisting, the check if (typeof module !== "undefined" && module.require) to determine whether the script is running on Node creates a non-working bundle, because typeof module is replaced by a string and this if check also fails:

parcel/packages/core/parcel-bundler/src/scope-hoisting/hoist.js

Lines 98 to 107 in 636bb50

	if (
	path.node.name === 'module' &&
	(!path.parentPath.isMemberExpression() || path.parent.computed) &&
	!(
	path.parentPath.isUnaryExpression() &&
	path.parent.operator === 'typeof'
	) &&
	!path.scope.hasBinding('module') &&
	!path.scope.getData('shouldWrap')
	) {

This causes the asset to not be wrapped and so module isn't being declared (causing Uncaught ReferenceError: module is not defined)

This replace module.require with null in the browser (as is done for server other module properties).

Closes #2841

🚨 Test instructions

Not sure how to add this as a test, because module is actually being declared as a global by run test function.

//index.js
const {getEnv} = require("./env.js");
console.log(getEnv());

//env.js
exports.getEnv = function () {
	if (typeof module !== "undefined" && module.require) {
		return "Node";
	} else {
		return "Browser";
	}
};

REPL Link (Open console to see the error as well)

✔️ PR Todo

• Added/updated unit tests for this change
• Filled out test instructions (In case there aren't any unit tests)
• Included links to related issues/PRs

[mischnic]

This replace module.require with null in the browser

I'm not even sure if that's what we want to do. The "normal" require was already parsed by the hoister

Is if (typeof module !== "undefined" && module.require) the "canonical" way to check for Node?

module.require(...) might not work with this because if during hoisting the require wasn't possible to parse, then this would produce an error null(...)

[devongovett]

Could you add a test?

[devongovett]

module.require appears not to be implemented by webpack, so I'd say replacing it with null is fine.

[mischnic]

@devongovett You did what I mentioned in my last comment:
adding a test as you did passes even without the changes to hoist.js because module is actually defined in the test environment:

parcel/packages/core/test-utils/src/utils.js

Lines 230 to 238 in a50ddd5

	var ctx = Object.assign(
	{
	exports,
	module: {exports},
	document: fakeDocument,
	WebSocket,
	console,
	location: {hostname: 'localhost', reload() {}},
	fetch(url) {