Rename all fonts to use their names in react-native-vector-icons?

[gsklee]

Okay, so, this is actually the same issue as #9; it's not being resolved by a PR, the issue is still there, I'm using the latest CRNA/expo(v17) combo.

I think this issue can be easily mitigated by renaming all fonts to use their names inside the original react-native-vector-icons; ie. you will wanna change this from 'material' to 'Material Icons', etc. etc.

May we discuss why their names in this repo deviated from their original names and, if it will cause any harm to change them back? Thanks!

[brentvatne]

hi @gsklee! Sorry that this caused problems for you. It's not clear to me that the problem you are encountering is related to the name of the fonts -- I can't imagine why they would be used outside of the vector-icons libraries, and if so, you can get the name of the font rather than use an arbitrary string by doing Ionicons.expoFontName (we should probably change this to just fontName), for example. I'd be happy to change them do whatever react-native-vector-icons uses if I thought it would help.

The reason why @expo/vector-icons exists is that we use a different mechanism for loading fonts than a normal React Native app -- with Expo we need to load them from a disk cache or the web, whereas they are included in the app bundle when you build a React Native app with Xcode/Android Studio. So @expo/vector-icons takes care of loading fonts properly and, if they aren't available on disk, delaying displaying the icon until the font has loaded.

Of course we can't ask every library to depend on @expo/vector-icons and so many libraries depend on react-native-vector-icons, so I aliased react-native-vector-icons to @expo/vector-icons in the expo babel preset. This is not an ideal situation, and my plan eventually is to get rid of a need for @expo/vector-icons and babel-preset-expo entirely. There is an issue related to this on CRNA: expo/create-react-native-app#96

I hope that helps give some background on the situation, but it's possible that it won't solve the immediate issue! Are you able to provide me with access to the project where you are experiencing this problem so I can investigate further? Or to reproduce within another example project? Thanks!

[gsklee]

Hi @brentvatne, I appreciate the detailed explanation, but it answered a question I didn't ask; my issue is exactly the same with #9. To illustrate:

This is a very simple react-native-elements component:

import { SearchBar } from 'react-native-elements';

// ...
render () {
  return (
    <SearchBar/>
  );
}
// ...

It will result in the error reported in #9:

It won't work if I do the standard preloading mechanism as suggested in your docs; the same error still appears:

  async componentDidMount () {
    const {navigation} = this.props;

    await Promise.all([
      ...cacheImages(Object.values(images)),
      ...cacheFonts([MaterialIcons.font])
    ]);

    navigation.navigate('Main');
  }

It works if I do this instead:

  async componentDidMount () {
    const {navigation} = this.props;

    await Promise.all([
      ...cacheImages(Object.values(images)),
      ...cacheFonts([
        {'Material Icons': MaterialIcons.font['material']}
      ])
    ]);

    navigation.navigate('Main');
  }

The culprit, as I've explained, is that you changed the name of the fonts for no apparent reason. While it's 'Material Icons' in react-native-vector-icons, you called it 'material'; while it's 'FontAwesome' in react-native-vector-icons, you called it 'awesome', etc. etc. So despite you aliasing react-native-vector-icons to @expo/vector-icons, it doesn't actually work as you intended because of the font names mismatching.

The fastest way to fix this issue, I believe, is to simply change the font names to align with the names inside react-native-vector-icons. And I was asking that if this change would cause any negative effect. If not, then a simple PR will be able to fix this issue.

I'd also suggest you to spend some time and play around react-native-elements to see the compatibility issue yourself since it's kinda like the semi de facto community components library around the place at this moment.

[brentvatne]

Hi @brentvatne, I appreciate the detailed explanation, but it answered a question I didn't ask; my issue is exactly the same with #9.

I believe it did -- I explained that I did not see there being any difference in renaming the fonts. Like I said I'd be happy to do it if I thought it would help. I think the error you're seeing is because of another problem, though.

This is a very simple react-native-elements component:
It will result in the error reported in #9:

It does not result in this error. I just created a new project and added SearchBar and did not get any errors.

It works if I do this instead:
// {'Material Icons': MaterialIcons.font['material']}

The culprit, as I've explained, is that you changed the name of the fonts for no apparent reason. > While it's 'Material Icons' in react-native-vector-icons, you called it 'material'; while it's 'FontAwesome' in react-native-vector-icons, you called it 'awesome', etc. etc. So despite you aliasing react-native-vector-icons to @expo/vector-icons, it doesn't actually work as you intended because of the font names mismatching.

This is incorrect, as far as I can tell. Let's look at @expo/vector-icons MaterialIcons.js

import glyphMap
  from './vendor/react-native-vector-icons/glyphmaps/MaterialIcons.json';
import createIconSet from './createIconSet';

export default createIconSet(
  glyphMap,
  'material',
  require('./fonts/MaterialIcons.ttf')
);

now let's look at Expo's createIconSet

export default function(glyphMap, fontName, expoAssetId) {
  const expoFontName = Font.style(fontName, {
    ignoreWarning: true,
  }).fontFamily;
  const font = { [fontName]: expoAssetId };
  const RNVIconComponent = createIconSet(glyphMap, expoFontName);
  // etc..

Notice that it uses the fontName that is passed in when creating the vector icon component.

Now, with the alias, when you do import Icon from 'react-native-vector-icons/MaterialIcons', what you get is the @expo/vector-icons code that I showed above, which calls into createIconSet and uses the font name 'material'. So this import from react-native-elements would get aliased to use @expo/vector-icons/MaterialIcons.

The only way this would not work is if the babelrc is not configured to use babel-preset-expo. The error that you are seeing would occur in that situation -- it's trying to use 'Material Icons' as the fontFamily, which means it must be using react-native-vector-icons/MaterialIcons rather than @expo/vector-icons/MaterialIcons.

I'd also suggest you to spend some time and play around react-native-elements to see the compatibility issue yourself since it's kinda like the semi de facto community components library around the place at this moment.

I have done this and written up a guide in the past on how to integrate, and helped them to become compatible in various ways. Everything works as expected unless the babel preset isn't properly configured.

[brentvatne]

The fastest way to fix this issue, I believe, is to simply change the font names to align with the names inside react-native-vector-icons. And I was asking that if this change would cause any negative effect. If not, then a simple PR will be able to fix this issue.

It would not cause any problems, except that it might mask an underlying configuration issue where the Expo preset is not properly set up. I would accept a PR for it nonetheless -- if you don't want to use the Expo preset then this would make it easier to do so!

[gsklee]

@brentvatne In that case I am no longer sure about the source of the issue because the project was initialized using CRNA and I didn't touch anything about Babel configs.

[gsklee]

I'd like to say, however, that using Babel transformation to tackle this issue both made the source code harder to understand (what is described by the source code is not true and there are some meta mechanisms that I need to also become aware of to properly interpret the code) and harder to debug. Maybe you should figure out a better way to do this.

[brentvatne]

I'd like to say, however, that using Babel transformation to tackle this issue both made the source code harder to understand (what is described by the source code is not true and there are some meta mechanisms that I need to also become aware of to properly interpret the code) and harder to debug. Maybe you should figure out a better way to do this.

I agree! I explained the idea for how we can do this above in my original reply. If you have time to help with that it would be very welcome; we have a lot to do at Expo and are a small team still so any help is appreciated.

[brentvatne]

Again @gsklee if you share the project with me I am happy to help you get to the bottom of it. I have no idea what could be causing this if it's not the babel config. Did you eject the project, by any chance? I'd be happy to sign a NDA or whatever if that's necessary for accessing the project.

[gsklee]

@brentvatne I can help after this next week, have a tight deadline to meet right now 😭

[brentvatne]

@gsklee - good luck! :D

[Mnabawy]

@gsklee me too