paralleldrive / cuid

Collision-resistant ids optimized for horizontal scaling and performance.
Other
3.44k stars 123 forks source link

navigator is not defined #40

Closed ghost closed 7 years ago

ghost commented 9 years ago

I could being doing something really stupid (as is the case normally) but I can't get my tests to pass in Node with cuid.

error trying to resolve DateInputViewModel navigator is not defined ReferenceError: navigator is not d
efined

If I dig a little further, it looks like cuid is expecting the window.navigator to exist... but obviously this is not the case when running in a Node process. I am using Testem's launchers to run this particular suite of tests, and perhaps that is where the error lies. Maybe Testem isn't setting an environment variable correctly and therefore some kind of cuid check of if process.env === node is failing?

Any ideas would be appreciated.

ericelliott commented 9 years ago

What version are you using? Are you sure it's cuid causing the error?

The node version of cuid shouldn't give you any trouble with browser stuff.

ghost commented 9 years ago

"cuid": "^1.2.4" and node v0.12.7... I'll double check again where the error is coming from.

ericelliott commented 9 years ago

The latest version of cuid is 1.2.5. Try that, first.

ghost commented 9 years ago

1.2.5 Didn't resolve the problem, either. Maybe something else is conflating what's really going on. I'm trying to run tests via mochajs - and that's where the navigator is not defined error's are coming from. This is expected, since node processes don't have a window.navigator - but I thought cuid supported node and browsers... which is what makes me think something is tricking cuid into believing it's actually inside a browser.

Interestingly I was able to reacreate the problem outside of my app. I created a my-test.js file that looks like this...

describe('work', function () {
    var cuid = require('cuid')
    it('should work', function() {
        var canevaluate = typeof navigator == "undefined";
        var id = cuid()
        console.log('ID', id)
        console.log('canEval', canevaluate)
        expect(id).to.equal(id)
    })
})

1) run browserify -t babelify -e ./spec-support/index.js my-test.js -o compiled.js 2) run mocha compiled.js

  work
    1) should work

  0 passing (9ms)
  1 failing

  1) work should work:
     ReferenceError: navigator is not defined
      at Function.browserPrint (specs.js:12825:17)
      at cuid (specs.js:12784:27)
      at Context.<anonymous> (specs.js:8:18)

Does this mean the browserify pre-compile step is evaluated as "hey we're in the browser, navigator is here... no worries" and then chokes when the code is evaluated again inside node? On another note - the latest version is 1.2.5, yet github doesn't have that release tagged? I'm seeing a strange difference in package.json files too.

github/cuid/package.json

  "version": "1.2.5",
  "main": "./build/server/cuid.js",
  "browserify": "./build/client/cuid.js",

node_modules/cuid/package.json

  "version": "1.2.5",
  "main": "./dist/node-cuid.js",
  "browserify": "./dist/browser-cuid.js",
ericelliott commented 9 years ago

Interesting. It looks to me like the server version isn't even getting built. @therealklanni

ericelliott commented 9 years ago

I strongly suspect this is related to issue 42. Does the current version work for you?

ghost commented 9 years ago

It doesn't, but the issue may very well still be on my end. I'm thinking the problem is that when running browserify to compact all the **/tests/*-spec.js the environment is node set to --node, which means your package.json "browser": "/dist/browser-cuid.js" is getting pulled in instead. Bummer.

therealklanni commented 9 years ago

@ericelliott looking into it. This was working before, might be related to the recent changes, trying to nail it down.

In short, the "navigator is not defined" is caused by the fact that node doesn't have a concept of a navigator, so for tests to run they need to be run in a browser, not in node.

ericelliott commented 9 years ago

@therealklanni Yeah, but in a Node build, the "main" module should be used, not the "browser" version. The browser version should only be pulled in for browserify / webpack builds targeting browsers. AFAIK, this is configured correctly in our package.json, so I'm a little puzzled about why this is happening.

Here's an ugly workaround in the meantime: specify the exact path/filename to import the correct package instead of import cuid from 'cuid';.

therealklanni commented 9 years ago

I think this is related to #42. I believe once we finish resolving that issue it will shed some light on this.

jdoleary commented 7 years ago

I just ran into this issue trying to use the new node debugger built into Chrome Dev Tools

ericelliott commented 7 years ago

If you want to try to get resolve the build/CI test failures in this PR it should solve this issue.