Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'global is not defined' error when 'uglify-js' required into test suite #1719

Closed
Akurganow opened this issue Mar 28, 2017 · 13 comments · Fixed by #1720
Closed

'global is not defined' error when 'uglify-js' required into test suite #1719

Akurganow opened this issue Mar 28, 2017 · 13 comments · Fixed by #1720
Labels

Comments

@Akurganow
Copy link

Packages:

  • uglify-js@2.8.16
  • jest@19.0.2

__tests__/test.js:

const buildStatic = require('../build-static')

test('empty test', () => {
  expect(true).toBe(true)
})

build-static.js:

const UglifyJS = require('uglify-js')

module.exports = function() {
  // Empty function
}

and when i run tests got this error

FAIL  __tests__/test.js

Test suite failed to run

ReferenceError: global is not defined

    at eval (eval at Object.<anonymous> (node_modules/uglify-js/tools/node.js:27:1), <anonymous>:10093:5)
    at Object.<anonymous> (node_modules/uglify-js/tools/node.js:29:17)
    at Object.<anonymous> (build-static.js:1:16)
    at Object.<anonymous> (__tests__/test.js:1:19)
    at process._tickCallback (internal/process/next_tick.js:109:7)
@alexlamsl
Copy link
Collaborator

Are you running on Node.js? If so, global has been around since forever.

@Akurganow
Copy link
Author

I'm running jest from npm scripts, node version 7.7.4

@kzc
Copy link
Contributor

kzc commented Mar 28, 2017

@alexlamsl Can you revert this recent change in tools/exports.js?

-if (typeof DEBUG !== "undefined" && DEBUG) {
+if (global.UGLIFY_DEBUG) {

global won't work when uglify is run in a browser, and it's not the correct way to see if a global variable is defined.

@alexlamsl
Copy link
Collaborator

If you are running uglify-js under Node.js, global will always be defined. You'll have to investigate what jest does which seems to break that.

@kzc we don't use tools/export.js in the browser though, do we?

@kzc
Copy link
Contributor

kzc commented Mar 28, 2017

Trust me on this. There was another bug related to this same line.

@alexlamsl
Copy link
Collaborator

I thought the whole point of tools/node.js is to be run under Node.js? I would very much like to know why this doesn't work.

Either case, this particular flag should probably go altogether. There is no reason to export this over just one mocha test.

@kzc
Copy link
Contributor

kzc commented Mar 28, 2017

we don't use tools/export.js in the browser though, do we?

Some do. After running the result of uglifyjs --self.

There's also a pending PR to get minify() working in the browser.

@kzc
Copy link
Contributor

kzc commented Mar 28, 2017

$ bin/uglifyjs --self -b | grep global | grep export
(function(exports, global) {
            var wrapped_tl = "(function(exports, global){ '$ORIG'; '$EXPORTS'; global['" + name + "'] = exports; }({}, (function(){return this}())))";
    global["UglifyJS"] = exports;

@alexlamsl
Copy link
Collaborator

Oh I see - they don't use tools/node.js but they do include tools/exports.js

I'll spin up a PR to get rid of UGLIFY_DEBUG in a moment.

@kzc
Copy link
Contributor

kzc commented Mar 28, 2017

Either case, this particular flag should probably go altogether. There is no reason to export this over just one mocha test

That's your call. Here's the issue in question: #1148

commit aa82027a1721ee7233e92ab1b9f9ced43a8f17cf
Author: Richard van Velzen
Date:   Mon Jun 20 08:39:46 2016 +0200

    Don't assume DEBUG is defined when exporting --self
    
    Potential fix for #1148
$ git diff aa82027a1721ee7233e92ab1b9f9ced43a8f17cf^!
diff --git a/tools/exports.js b/tools/exports.js
index a481143..54aa23e 100644
--- a/tools/exports.js
+++ b/tools/exports.js
@@ -18,6 +18,6 @@ exports["tokenizer"] = tokenizer;
 exports["is_identifier"] = is_identifier;
 exports["SymbolDef"] = SymbolDef;
 
-if (DEBUG) {
+if (typeof DEBUG !== "undefined" && DEBUG) {
     exports["EXPECT_DIRECTIVE"] = EXPECT_DIRECTIVE;
 }

@kzc
Copy link
Contributor

kzc commented Mar 28, 2017

@alexlamsl You could just export this unconditionally in tools/exports.js I suppose:

    exports["EXPECT_DIRECTIVE"] = EXPECT_DIRECTIVE;

I don't know what the intention of this was:

./test/mocha/directives.js:    it("Should test EXPECT_DIRECTIVE RegExp", function() {
./test/mocha/directives.js:            assert.strictEqual(uglify.EXPECT_DIRECTIVE.test(tests[i][0]), tests[i][1], tests[i][0]);
    it("Should test EXPECT_DIRECTIVE RegExp", function() {
        var tests = [
            ["", true],
            ["'test';", true],
            ["'test';;", true],
            ["'tests';\n", true],
            ["'tests'", false],
            ["'tests';   \n\t", true],
            ["'tests';\n\n", true],
            ["\n\n\"use strict\";\n\n", true]
        ];

        for (var i = 0; i < tests.length; i++) {
            assert.strictEqual(uglify.EXPECT_DIRECTIVE.test(tests[i][0]), tests[i][1], tests[i][0]);
        }
    });

@kzc
Copy link
Contributor

kzc commented Mar 28, 2017

Or just drop not export EXPECT_DIRECTIVE and drop the mocha test.

(edited)

alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Mar 28, 2017
@alexlamsl
Copy link
Collaborator

No worries, I have a workaround that removes the debug flag whilst still exercise the RegExp in question.

@alexlamsl alexlamsl added the bug label Mar 28, 2017
alexlamsl added a commit that referenced this issue Mar 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants