fix 'location is not defined' error

[jakemmarsh]

I receive the error ReferenceError: location is not defined when running Mocha tests via Gulp. I'm using jsdom to mock the document and window, which is resulting in the global location being undefined unless I explicitly do global.location = '{}; (or utilize this fix).

Fix of PR #1653

CC @dasilvacontin

[dasilvacontin]

Nice! It wasn't necessary to create a new Pull Request though, they get updated when you commit to the same branch. That way it's easier to keep up the conversation/discussion. :)

[jakemmarsh]

I foolishly hadn't created a separate branch for the other PR, which is why I've created this new one. Should be good to go! 👍

[dasilvacontin]

Oh, my bad then.

[dasilvacontin]

Doesn't this still give you a ReferenceError, since you are trying to access location first? Sorry about my misleading example, it's not as easy as accessing window.location since window might not exist.

Also, commits should be squashed into a single one if they are part of the same change.

[dasilvacontin]

I mean, if you check for location first, you still have the same issue in your case.
And if we check by window.location first, window might not exist in scenarios like yours.

[jakemmarsh]

Squashed commits, and added better checking so it should work in both cases. In my case, window isn't undefined because I'm using jsdom to mock it. However, it just seems that using jsdom to mock the window doesn't automatically globalize location.

[dasilvacontin]

Cool, LGTM! Thanks for the PR 😄

However, it's breaking the build.

[boneskull]

@jakemmarsh Sorry, something is odd--there are updates to package.json and HISTORY.md in here which shouldn't be.

[dasilvacontin]

LGTM and the build works now. If you squash we'll merge. :) Actually, we are still discussing something.

[a8m]

Note that these double-brackets unnecessary here.

[epelz]

Any update on this fix?

[boneskull]

@jakemmarsh able to make the changes requested by @a8m?

[jakemmarsh]

@boneskull made the updates and squashed to 1 commit!

[a8m]

LGTM!
Thanks @jakemmarsh

[epelz]

@boneskull Is this ready to merge now?

[dasilvacontin]

LGTM. Thanks for the PR, @jakemmarsh. :)

[akfish]

This issue is still present in some wired environments. I ran into it in atom editor's Task today. In the context of atom task, document object is present while location/window.location is not.
I know this might be a corner case here. But how about a more permanent fix, something like checking process and location directly and use whichever is available?

[l0b0]

I ran into this issue when running make test in https://github.com/l0b0/insecure-links-highlighter/tree/2d737d25b1ec4b1d820e85a442a5cc731a27c696 after adding a reference to location in highlight.js: exports.protocol = location.protocol;

Relevant output:

docker run --rm insecure-links-highlighter-nodejs /build/node_modules/.bin/mocha test/unit
/project/highlight.js:16
    exports.protocol = location.protocol;
                       ^

ReferenceError: location is not defined
    at /project/highlight.js:16:24
    at Object.<anonymous> (/project/highlight.js:54:2)
    at Module._compile (module.js:660:30)
    at Object.Module._extensions..js (module.js:671:10)
    at Module.load (module.js:573:32)
    at tryModuleLoad (module.js:513:12)
    at Function.Module._load (module.js:505:3)
    at Module.require (module.js:604:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/project/test/unit/test.js:4:17)
    at Module._compile (module.js:660:30)
    at Object.Module._extensions..js (module.js:671:10)
    at Module.load (module.js:573:32)
    at tryModuleLoad (module.js:513:12)
    at Function.Module._load (module.js:505:3)
    at Module.require (module.js:604:17)
    at require (internal/module.js:11:18)
    at /build/node_modules/mocha/lib/mocha.js:231:27
    at Array.forEach (<anonymous>)
    at Mocha.loadFiles (/build/node_modules/mocha/lib/mocha.js:228:14)
    at Mocha.run (/build/node_modules/mocha/lib/mocha.js:536:10)
    at Object.<anonymous> (/build/node_modules/mocha/bin/_mocha:582:18)
    at Module._compile (module.js:660:30)
    at Object.Module._extensions..js (module.js:671:10)
    at Module.load (module.js:573:32)
    at tryModuleLoad (module.js:513:12)
    at Function.Module._load (module.js:505:3)
    at Function.Module.runMain (module.js:701:10)
    at startup (bootstrap_node.js:190:16)
    at bootstrap_node.js:662:3