Platform.select method

[grabbou]

Platform.select, given an object containing Platform.OS as keys, returns the value for the platform you are currently running on.

It works with styles:

var styles = StyleSheet.create({
  container: {
    flex: 1,
    ...Platform.select({
      ios: {
        backgroundColor: 'red',
      },
      android: {
        backgroundColor: 'blue',
      },
    }),
  },
});

as well as with functions:

var Component = Platform.select({
  ios: () => require('ComponentIOS'),
  android: () => require('ComponentAndroid'),
})();

<Component />;

The entire implementation :)

 var Platform = {
   OS: 'ios',
+  select: (obj: Object) => obj.ios,
 };

Kudos to @frantic for this amazing idea suggested in #7033

[ide]

Think we don't want this because you should be able to run code like this:

Platform.select({
  android: { ... }
})

and have the Android style get filtered out on iOS even if there are no iOS-specific styles.

[grabbou]

That's true, however the good thing is it can be used with non-style values and also at any level of StyleSheet object.

That means it's tricky to return a default value when Platform key is missing, like in the below example:

// will throw on Android, we can return an `{}` as a default, but then, the next example wont' work
Stylesheet.create({
   ...Platform.select({ 
      ios: {} 
   }) 
})

and

// here it might work, but it might be unexpected
StyleSheet.create({
   container: { 
     marginTop: Platform.select({
        ios: 5,
     })
   } 
})

and forgot to specify another platform.

My initial reasoning was that this is intended to be used when you have values for both platforms to support - otherwise you can just supply an empty object.

PS. I have just noticed that it's going to also throw when value is falsy (e.g. 0 given for marginTop, so that will have to be handled as well)

[vjeux]

Agreed with @ide, I would remove that invariant. Not specifying an android block if you are on ios is okay.

[ghost]

By analyzing the blame information on this pull request, we identified @emilioicai and @knowbody to be potential reviewers.

[vjeux]

Remove this one as well. This is going to be a hot path and the error isn't very useful.

[vjeux]

This is going to be the most useful dummy one liner I've seen in a while :)

[ide]

Can you write jest tests too?

[grabbou]

@ide sure, doing that now. Shall I also add a simple test for OS property?

[vjeux]

This is probably not going to work as you would expect. You're going to require the Android component on iOS and it may very well throw in this environment.

You'll likely want to write this as

var Component = Platform.select({
  ios: () => require('ComponentIOS'),
  android: () => require('ComponentAndroid'),
})();
return <Component />;

[vjeux]

It cannot hurt :)

[vjeux]

I'm happy with the diff. Any other input @ide?

[vjeux]

actually, remove this, you're not doing the same for StyleSheet

[grabbou]

Would you mind clarifying? (Is the PR update allowed when shipping in progress?)

[vjeux]

Just remove this line, we would assume that people know how to require Platform.

For a shipping in progress, it'll only be picked up if I re-shipit.

We can just leave it for now, or kill it in another commit once it lands

[ide]

🚢 🇮🇹

[vjeux]

@facebook-github-bot shipit

[vjeux]

Could you update the pull request description with the two examples that you added in the doc. I want to tweet about this and it'll make for a better landing page :)

[ghost]

Thanks for importing. If you are an FB employee go to Phabricator to review.

[grabbou]

All righty. Should be done now.

[sebmarkbage]

Does anyone have a good plan for dead code eliminating the unused platform?

[satya164]

@sebmarkbage May be someone could write a babel plugin transform this code at build time instead of runtime, so that

Platform.select({
  ios: {
    backgroundColor: 'red',
  },
  android: {
    backgroundColor: 'blue',
  },
}),

becomes

{
  backgroundColor: 'red',
},

[jamiebuilds]

@satya164 here you go https://astexplorer.net/#/o5XwSweFRa

[satya164]

Awesome @thejameskyle :D

cc @skevy should we include in our preset?

[abalhier]

Hello,

This feature seems great. Though I'm wondering why the way platform specific css rules is defined in the F8 app was not preferred :

var styles = StyleSheet.create({
  carousel: {
    ios: {
      margin: 10,
      overflow: 'visible',
      backgroundColor: 'black',
    },
  }
});

In F8StyleSheet.js :

export function create(styles: Object): {[name: string]: number} {
  const platformStyles = {};
  Object.keys(styles).forEach((name) => {
    let {ios, android, ...style} = {...styles[name]};
    if (ios && Platform.OS === 'ios') {
      style = {...style, ...ios};
    }
    if (android && Platform.OS === 'android') {
      style = {...style, ...android};
    }
    platformStyles[name] = style;
  });
  return StyleSheet.create(platformStyles);
}

From a user's perspective it would seem like there is less boilerplate code to write compared to using Platform.select and a spread operator. Do you think it's worth creating a pull request for this ?

[Bhullnatik]

@abalhier You have the discussion leading to this decision on this PR : #7033

[abalhier]

Thank you @Bhullnatik :)

[vjeux]

The gist of it is that we are trying to have decoupled concepts as much as possible. We don't want to tie in platform specific behavior with stylesheets. Chosing different things to do based on the platform is not limited to stylesheets and we'd rather have a specific api for it.

Now, we also need to make sure that developer experience is taken into account and verbosity is an important piece of it

[ide]

Also this is something that is easy to do in user space without changes to the RN core so if you like F8StyleSheet's pattern, you should reimplement it and publish it to npm instead of sending PRs.

[knowbody]

so I created this: https://github.com/knowbody/react-native-platform-stylesheet and published to npm so you can just: npm install react-native-platform-stylesheet