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

Make it works on React Native #460

Closed
wants to merge 2 commits into from
Closed

Conversation

ShongSu
Copy link

@ShongSu ShongSu commented May 17, 2017

No description provided.

@coveralls
Copy link

coveralls commented May 17, 2017

Coverage Status

Coverage remained the same at 63.804% when pulling 38c5bbe on ShongSu:master into 6bb07f7 on visionmedia:master.

@suark
Copy link

suark commented May 17, 2017

We just had an issue with this in our React-Native project. I manually implemented your change in this PR and it worked for me.

@@ -40,7 +40,7 @@ function useColors() {
// NB: In an Electron preload script, document will be defined but not fully
// initialized. Since we know we're in Chrome, we'll just detect this case
// explicitly
if (window && window.process && window.process.type === 'renderer') {
if (window && window.process && window.process.type === 'renderer' || !window.document) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really the best feature-detection for react-native though? Seems fragile.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we can do navigator.product === 'ReactNative' too.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the point wasn't just to detect react-native, but to detect a lack of window.document which is used in the next part of the check.

@RangerMauve
Copy link

I'm guessing #462 fixes this so it should be closed?

@thebigredgeek
Copy link
Contributor

I believe fixed #462. Closing

@glocore
Copy link

glocore commented Aug 5, 2018

Hi, how exactly would this work? I'm not able to see the logs on the browser running the React Native debugger.

Copy link
Member

@Qix- Qix- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with @TooTallNate, seems wrong. I'm very skeptical of adding these sorts of checks because platforms should be providing a standardized way to do this.

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

Successfully merging this pull request may close these issues.

8 participants