NullReferenceException and expected identifiers/expressions with const+module.exports

[Jman012]

I am getting two errors that are unexpected. One is a NullReferenceException, and the other is "Expected expression: ." and "Expected identifier: ." in a couple scenarios.

I've been able to narrow down the issue to make small snippets.

First, a passing scenario:

var test = "1";
function test(module) {
	module.exports = function () {

	};
}

This compiles to function test(n){n.exports=function(){}}var test="1";.

Failure 1: NullReferenceException
Change var test to const test.

const test = "1";
function test(module) {
	module.exports = function () {

	};
}

I get a NullReferenceException in https://github.com/xoofx/NUglify/blob/v1.5.14/src/NUglify/JavaScript/Syntax/MemberExpression.cs#L70 because Root is null.

Failure 2: Expected expression/identifier
Change function () { } to "1".

const test = "1";
function test(module) {
	module.exports = "1";
}

The location of these two errors are on the dot after module.

As far as I know, neither module nor exports are not reserved keywords in JavaScript. I am not sure what is going wrong here. This particular issue was with the combination of a plain .js file that happens to use const here and there, bundled with (see: BundleBuilderMinifier) a .min.js file that happens to use module and exports as function parameters/arguments.

The solution for now is to not use const. I was under the impression that this wasn't a new JS addition, and that it would make something like NUglify parse it any different. We are moving away from a runtime-bundling of our files with Microsoft.Ajax.Utilities.Minifier and instead using build-time-bundling with BundleBuilderMinifier, and this may cause issues with certain bundles.

Any help would be appreciated!

[devIceMan]

Same here. If you can switch from package to code, then for a quick workaround you can replace the body of the first if statement at NUglify.JavaScript.JSParser.PeekCanBeModule:

original:

if (ParsedVersion == ScriptVersion.EcmaScript6 || m_settings.ScriptVersion == ScriptVersion.EcmaScript6){
return true;
}

replacement:

if (ParsedVersion == ScriptVersion.EcmaScript6 || m_settings.ScriptVersion == ScriptVersion.EcmaScript6)
            {
                var clone = m_scanner.Clone();
                clone.SuppressErrors = true;
                var next = clone.ScanNextToken();

                if (next?.HasCode == true && next?.Code == ".")
                {
                    next = clone.ScanNextToken();
                    if (next?.HasCode == true && next?.Code == "exports")
                    {
                        return false;
                    }
                }
                
                return true;
            }

[Jman012]

Ah, then it looks like NUglify is parsing out the ECMA version before going further. That makes sense. The bundle I was trying to minify with BuildBundlerMinifier had one file that was essentially vendor code that was pre-compiled into lower ECMA code and then pre-minified, and another file that was our raw JS code that was only minified.

One solution would be to split these into different bundles, or at least more strongly define ECMA versions and keep bundles at similar versions.

However, I think that a NullReferenceException is still a red flag here.

[trullock]

This looks like it relates to #80

[trullock]

const test = "1";
function test(module) {
	module.exports = function () {

	};
}

isn't valid JS, because you shouldn't be redefining test.

Admittedly NUglify shouldn't explode because of this, so I'll try and fix that.

[trullock]

This also fixes cases like this

(function (factory) {
  if (typeof define === 'function' && define.amd) {
    // AMD. Register as an anonymous module.
    define(['a'], factory);
  } else if (typeof module === 'object' && module.exports) {
    // Node/CommonJS
    module.exports = function (a) {
      return a;
    };
  } else {
    // Browser globals
    factory(a);
  }
})

[trullock]

@devIceMan Is there any need for this in your code

                    next = clone.ScanNextToken();
                    if (next?.HasCode == true && next?.Code == "exports")
                    {
                        return false;
                    }

Shouldnt that whole block just be return false?

i.e. if you have module.<ANYTHING> then its return false?

It seems any variable named module followed by .anything causes upset

[devIceMan]

@trullock yes, you right - in "module.ANYTHING" situation it should returning false without any futher check

[trullock]

@trullock yes, you right - in "module.ANYTHING" situation it should returning false without any futher check

Agree, just pushed 1.6.3 with this change in