search

tejacques / crosstab

A utility library for cross-tab communication using localStorage.
Apache License 2.0
365 stars 58 forks source link

Use context variable rather than direct references to window/navigator #36

Closed chadly closed 9 years ago

chadly commented 9 years ago

I'm running mocha tests in my app where I use crosstab. I run the mocha tests with the CLI without a browser, e.g. mocha tests where tests is the folder with all my tests in it.

Crosstab fails the tests due to it accessing navigator and window directly. It complains about navigator being undefined. If it could just get past this step, it wouldn't be a problem as I am using sinon to mock the class that is using crosstab.

Anyway, I updated the UMD wrapper in crosstab to pass the current context (this - which will be global in node and window in the browser) down so that crosstab can get past that initial useragent check without blowing up when run from node.

tejacques commented 9 years ago

This is great and I like the idea, but I have one suggestion/change. Rather than naming the argument root, just name it window.

This serves two purposes.

  1. It keeps the intention clear that this is meant to be the window object in the browser (despite being global when run from the node CLI)
  2. It will be a smaller and safer change. An example of this is that there's still a reference to window in your update here.
chadly commented 9 years ago

Ahh, nice catch. Yeah, that is a better idea. I renamed root to window.

tejacques commented 9 years ago

Looks good, merging now. Will push to npm tonight.

tejacques commented 9 years ago

Latest version with these updates is now up on npm -- v0.2.8

chadly commented 9 years ago

Thanks!