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

[Idea: JS] Add way to detect react-native (and platform) without require(). #1331

Closed
mikelehen opened this issue May 18, 2015 · 21 comments
Closed
Assignees
Labels
Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Resolution: Locked This issue was locked by the bot.

Comments

@mikelehen
Copy link

Motivation

I have a 3rd-party library that needs to work in react-native (as well as node.js, web, etc.). I want to detect when it's used in react-native, for analytics purposes. I can't use require('react-native') or similar because if somebody uses my library with a tool like browserify, it will try to statically satisfy all require() statements and fail on react-native.

Suggestion

  • Set navigator.product to 'ReactNative' (it is currently 'Gecko')
  • Set navigator.platform to 'iPhone', 'iPad', or 'Android' as appropriate (it is currently 'MacIntel', at least in the iOS emulator).

Implementing

Code in question is here:

Related IRC Conversation: https://gist.github.com/mikelehen/822a82a37bf30431a104

@sophiebits
Copy link
Contributor

I expect that navigator.platform is already something reasonable on device; I don't think we set it. Setting navigator.product sounds somewhat reasonable though…

@mikelehen
Copy link
Author

I'd be fine with dropping the .platform part. Somebody on IRC had mentioned it.

@ide
Copy link
Contributor

ide commented May 29, 2015

Proposal to move things forward: navigator.product returns "ReactNative" and navigator.productSub returns the version e.g. "0.5.0". This is a little different than browsers, which return "Gecko" and "20030107" but we'd be returning useful values in this case.

Relevant docs:
https://developer.mozilla.org/en-US/docs/Web/API/NavigatorID/product
https://developer.mozilla.org/en-US/docs/Web/API/Navigator/productSub

@ide ide added the Good first issue Interested in collaborating? Take a stab at fixing one of these issues. label May 29, 2015
@brentvatne brentvatne changed the title Feature: Add way to detect react-native (and platform) without require(). [Idea: JS] Add way to detect react-native (and platform) without require(). May 29, 2015
@mikelehen
Copy link
Author

FWIW, @ide's proposal would definitely work for our use case.

@oveddan
Copy link
Contributor

oveddan commented Jun 17, 2015

@spicyj @ide I have doubts that attaching something to the navigator will work in the chrome debugger, if we go ahead with the decision to run the chrome debugger in a worker (#1632 and #1473). The code within the worker doesn't have access to window, but just to the worker's own scope, which has I believe a different navigator. See the web worker's context api.

@ide
Copy link
Contributor

ide commented Jun 17, 2015

That sounds preferable actually. We don't want any of the browser environment to leak into React Native's environment ideally.

@rainchen
Copy link

+1 for this idea

@chetstone
Copy link
Contributor

It appears that attaching something to navigator is not going to work because navigator is read-only in webworkers. Does anyone have any other ideas on how to determine if code is running under react-native?

@chetstone
Copy link
Contributor

@mikelehen It seems that this idea is a nonstarter for the foreseeable future. Have you considered modifying the Firebase API to allow developers to tell you they're running under react-native?

I think an API change will be necessary anyway, because web code that relies on getAuth() being synchronous will probably break. There needs to be a way to pass a callback, either by an optional param to getAuth(), or adding a new call, say getAuthAsync(), to provide the asynchronous return value.

If a developer uses that API point, they're telling you they're using react-native. What do you say?

@vjeux
Copy link
Contributor

vjeux commented Nov 9, 2015

One thing we agreed on is to add a package.json rule called "react-native" that's going to let you change the root entry point of your module when npm install. The same way browserify, webpack and react-native support the "browser" field.

No one is working on it right now but if someone wants to send a diff i'll happily stamp it.

@ide
Copy link
Contributor

ide commented Nov 9, 2015

@vjeux this PR adds the react-native field: #2208. It needs to be rebased though.

@mikelehen
Copy link
Author

@chetstone Eh. I'm not excited about adding an API to indicate you're using React Native. It really needs to just work. FWIW, we already have an onAuth() method that's an async notification for auth status, so I'm not too worried about that. Astute observation about getAuth() though. :-)

The "react-native" package.json rule would actually work for us if that gets in. It's a pretty heavy-handed solution (we'll essentially just duplicate our "browser" client but add "var REACT_NATIVE = true;"), but whatever.

@zsszatmari
Copy link

I'd really need this as well. Is there a workaround-ish way today/now to check for react native? Preferably one which is not expected to break in the future?

@chetstone
Copy link
Contributor

@zsszatmari For library developers, probably not. For app developers, what I do is

var { Web } = require('../config/globals')

Then in globals.web.js I export Web = true and in globals.ios.js export Web = false

RN packager will pick up the .ios.js file if you're building for that platform. I use webpack for building the web app, so in my webpack.config I have

  resolve: {
    extensions: ['', '.web.js', '.js', '.jsx']
  },

so webpack will pick up the .web.js file.

@ide
Copy link
Contributor

ide commented Nov 10, 2015

Object.defineProperty works for setting navigator.product in Chrome (and should work in pure JSC, but not Safari), and @tadeuzagallo just killed the UIWebViewExecutor, so I think we can make navigator.product work. Would anyone like to send a PR?

@chetstone
Copy link
Contributor

I could maybe take a stab at it, but I have a couple of questions. First, what is UIWebViewExecutor and why is it relevant to setting a navigator property? I searched the issues but could not find any reference.

Second, is it OK if it doesn't work in Safari? @mikelehen you say it should just work. It could be confusing to devs if something fails in Safari. Perhaps the package.json fix would be more robust for you.

@ide
Copy link
Contributor

ide commented Nov 11, 2015

The web view executor used a UIWebView to run the JS. In Safari, though, you can't use defineProperty to set navigator.product because it is marked as non-configurable. But now the only two executors we have are JSC and Chrome.

@chetstone
Copy link
Contributor

So that means it doesn't matter if the navigator.product can't be set in Safari since we can't debug in Safari anyway (Debug in Safari has been removed from the "shake" menu?)

@ide
Copy link
Contributor

ide commented Nov 11, 2015

Yeah

ghost pushed a commit that referenced this issue Nov 25, 2015
Summary: Fix for [Issue 1331](#1331). Sets navigator.product to ReactNative and navigator.productSub to the version string in package.json.

Note that the code requires package.json, which works fine in the RN packager, but webpack users will probably a need to configure a json loader in their config file.

Tested using UIExplorer and console.log printout of the product variables in xcode and Chrome debugger.
Closes #4083

Reviewed By: svcscm

Differential Revision: D2696881

Pulled By: vjeux

fb-gh-sync-id: 511446432dcd0ec658100715129c77153e743423
sunnylqm pushed a commit to sunnylqm/react-native that referenced this issue Dec 2, 2015
Summary: Fix for [Issue 1331](facebook#1331). Sets navigator.product to ReactNative and navigator.productSub to the version string in package.json.

Note that the code requires package.json, which works fine in the RN packager, but webpack users will probably a need to configure a json loader in their config file.

Tested using UIExplorer and console.log printout of the product variables in xcode and Chrome debugger.
Closes facebook#4083

Reviewed By: svcscm

Differential Revision: D2696881

Pulled By: vjeux

fb-gh-sync-id: 511446432dcd0ec658100715129c77153e743423
Crash-- pushed a commit to Crash--/react-native that referenced this issue Dec 24, 2015
Summary: Fix for [Issue 1331](facebook#1331). Sets navigator.product to ReactNative and navigator.productSub to the version string in package.json.

Note that the code requires package.json, which works fine in the RN packager, but webpack users will probably a need to configure a json loader in their config file.

Tested using UIExplorer and console.log printout of the product variables in xcode and Chrome debugger.
Closes facebook#4083

Reviewed By: svcscm

Differential Revision: D2696881

Pulled By: vjeux

fb-gh-sync-id: 511446432dcd0ec658100715129c77153e743423
@davidLeonardi
Copy link

BUMP!
What's the status on the commits?
Can this issue be closed soon please?

@satya164
Copy link
Contributor

Fixed in 3c65e62

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests