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

Adding mobile fallback in dependency resolution #3641

Closed

Conversation

chrisnojima
Copy link
Contributor

This is more of a conversation starter. In our project we're starting to mix react-native and web. We want to effectively share ios and android so it's handy to have a file.mobile.js fallback in the require resolution in the packager. This turns into a 2 liner in the packager (sans documentation). Is there a better solution?

The idea is on iOS it looks for

file.ios.js
file.mobile.js
file.js
(file.json etc)

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @amasad, @martinbigio and @jspahrsummers to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Oct 23, 2015
@ide
Copy link
Contributor

ide commented Oct 26, 2015

@vjeux did you want this to be .web.js?

@vjeux
Copy link
Contributor

vjeux commented Oct 26, 2015

We already have support for .web.js, look at the platform block just above

@martinbigio
Copy link
Contributor

The more complicated dependency resolution becomes the more changes we'd need to make to other frameworks/tools that hook into React Native (i.e.: flow).

@chrisnojima
Copy link
Contributor Author

This is a PR for having android and ios fallback to mobile. web is blacklisted in RN already but that's unrelated

@ide
Copy link
Contributor

ide commented Oct 26, 2015

It's not clear to me that "mobile" is the right fallback. What about mobile web? Or desktop native? Tablet?

@chrisnojima
Copy link
Contributor Author

Any naming works for me. Having a simple static fallback path seems like an easy thing to add (can't speak to @martinbigio 's concerns above). Other option would be to make this configurable...

@vjeux
Copy link
Contributor

vjeux commented Oct 26, 2015

If you have Module.js and Module.ios.js, it'll take Module.js with no extension for android. This is the way you can do fallback.

If you need something more complicated, I suggest you write it in code with Platform.os ===

@chrisnojima
Copy link
Contributor Author

@vjeux yes, but I am working with android/ios/and web now. I often have android / ios code that is exactly the same but web code that is different. The thinking with the mobile fallback is for this case

Module.ios.js then Module.mobile.js then Module.js
Module.android.js then Module.mobile.js then Module.js
Module.web.js then Module.js

This seems like it'll be a common pattern. If not, I'm happy to just change my packager to work like this and not have this go upstream.

@satya164
Copy link
Contributor

@chrisnojima Won't this work?

Module.js // Android and iOS will take this
Module.web.js // Web will take this

@chrisnojima
Copy link
Contributor Author

@satya164 it will but we were looking to avoid this because of this problem.

We'll have a hierarchy where we have mostly platform agnostic files
like
/actions or /reducers

and more platform specific files
/containers and /components

if we implicitly fallback we'll have a scenario where in /actions the Module.js will be platform agnostic but in /containers Module.js will be android/ios specific. Maybe not a big deal but we're looking to avoid this kind of ambiguity

@satya164
Copy link
Contributor

@chrisnojima I still cannot understand, can you give an example?

If you mean that the dependency resolver should look for module.android.js then module.web.js then module.js, then I think it becomes more confusing. Ideally it should fail if there is no module.android.js or module.js when in iOS IMO. Makes it simpler to understand.

@chrisnojima
Copy link
Contributor Author

@satya164 sure. I'm not saying it should fallback from android to web. i'm saying it falls back from android to mobile. here is a full tree

actions/Hello.android.js (android)
actions/Hello.js (ios and web)
actions/Goodbye.web.js (web)
actions/Goodbye.mobile.js (android and ios)
actions/Lunchtime.js (android and ios and web)

widgets/Username.web.js (web)
widgets/Username.android.js (android)
widgets/Username.ios.js (ios)
widgets/Password.mobile.js (android and ios)
widgets/Password.web.js (web)
widgets/ZipCode.js (android and ios and web, likely won't exist since react-native and react components aren't the same at this point)

The scheme is:
If there is a platform specific, use it (ios/android/web)
If not there are 'aggregate' platforms which imply shared code. I'm just advocating for 'mobile' which captures ios+android. So Goodbye.mobile.js is used by android and ios.
If there isn't a platform specific file the plain .js is used (ZipCode.js)

Since react-native makes android and ios code likely very shareable and web likely isn't (at this point) the 'mobile' concept is very useful to me

My concern of having plain .js be the fallback is the if you look in the Module.js it won't be clear what kind of code you'd find in there. In actions above it'd be pure platform agnostic code (since it likely can be). That's great. But in widgets it likely won't ever be (ZipCode in this example is likely not going to be shared be all 3 platforms unless it delegates all its children out to other widgets) so most of the file will be of the form Module.mobile + Module.web or Module.ios Module.android Module.web

@satya164
Copy link
Contributor

@chrisnojima I now get what you're saying, but I don't like the idea of a single extension representing multiple platforms. Right now, it seems fine, since there are only two native platform and one web. But in future, when more platforms will be there (be it in the core, or the community, e.g.- someone is working on React Native for OS X, and someone was doing for Apple TV), it will be very confusing when a single extension represents multiple platforms.

@chrisnojima
Copy link
Contributor Author

@satya164 sure, this is why I suggested this could be configurable (but then it wouldn't be a 2 line change). So should i close this out?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants