Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Correctly detect browser vs node environment #63

Merged
merged 1 commit into from
Sep 25, 2015
Merged

Correctly detect browser vs node environment #63

merged 1 commit into from
Sep 25, 2015

Conversation

victorb
Copy link
Contributor

@victorb victorb commented Sep 19, 2015

Change conditional to not try to access the window object (which throws error when window doesn't exist), instead we check the typeof so it doesn't crash if window is not defined.

We can also safely assume that window.location exists if window exists.

This closes issue #62

@victorb
Copy link
Contributor Author

victorb commented Sep 19, 2015

Ok, I've run the tests locally, all of them are passing. In CircleCI, ls is stepping over the timeout, so the tests are failing. In Travis, we get some cryptic message TypeError: Cannot read property 'pkg' of null which I have no idea why that happens. @diasdavid care to take a look?

@victorb
Copy link
Contributor Author

victorb commented Sep 19, 2015

Ok, was able to reproduce the travis problem after updating dependencies.

It seems that commit 5c12f11d784498add132b3ebedefcd0dbc23ae2b in ipfs/node-ipfsd-ctl broke the module for both Mac and what Travis CI is using. What's weird is that master for this project is passing but only on Travis. Locally the build is broken, wondering if it's some Travis cache. What I can see, ipfs/node-ipfsd-ctl is also passing on master locally for me, but not in this project.

@victorb
Copy link
Contributor Author

victorb commented Sep 25, 2015

Submitted PR #64 to fix the tests which will make this PR green as well.

daviddias added a commit that referenced this pull request Sep 25, 2015
Correctly detect browser vs node environment
@daviddias daviddias merged commit 4befe32 into ipfs-inactive:master Sep 25, 2015
@RichardLitt
Copy link
Contributor

👍 Good catch.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants