oblador / react-native-vector-icons

Customizable Icons for React Native with support for image source and full styling.
https://oblador.github.io/react-native-vector-icons/
MIT License
17.42k stars 2.12k forks source link

Monorepo structure #856

Open oblador opened 6 years ago

oblador commented 6 years ago

Problem

In many cases this library is stuck between a rock and a hard place. Continuous breaking changes in React Native requires us to update the library in ways that might break backwards compatibility with older releases. This is annoying to users that just want a newer version of the icon sets and cannot upgrade React Native for some reason. Vice versa, users that want to upgrade this library to use newer versions of React Native might not want the updated icon sets which themselves might contain breaking changes.

Proposed solution

I think we should consider moving to a monorepo structure so we can separate updates to the core and updates to the icon sets. The icon sets versioning would ideally mirror their source counterparts to avoid confusion, we could potentially even have updates automatically generated as soon as a new version of their source package is released.

This would also be beneficial for the APK/IPA size for people that plainly just react-native link.

While it's still possible to do so today, I think this would open up to people maintaining and publishing their own icon sets instead of demanding inclusion to the core package.

Known issues

This would be a breaking change that people will hate us for.

While we could keep the react-native-vector-icons package for compatibility, some things like the font references in Xcode will probably break (although that should just be a react-native link away to fix).

Unsure how it affects the expo compatibility library. Ping @brentvatne

Related issues

https://github.com/oblador/react-native-vector-icons/issues/823 https://github.com/oblador/react-native-vector-icons/issues/835

oblador commented 6 years ago

Ping @hampustagerud @MoOx

MoOx commented 6 years ago

I am not relying on this lib anymore (and mostly use custom svgs + svgr). That said I think it's a good idea to move to a multi-packages structures. Not sure that people will hate you for that. You can still provide an old-school single package that consume others smaller packages.

hampustagerud commented 6 years ago

I think this is a good idea, it makes sense to split the package into smaller parts. Like @MoOx says, a single package could still be provided to maintain compatibility but it also adds a package that needs maintenance (maybe not that much though). The upgrade process could be fairly simple but there will probably be some annoyed people.

brentvatne commented 6 years ago

I think we should consider moving to a monorepo structure so we can separate updates to the core and updates to the icon sets. The icon sets versioning would ideally mirror their source counterparts to avoid confusion, we could potentially even have updates automatically generated as soon as a new version of their source package is released.

this makes a lot of sense to me. would each of the icon sets be shipped separately? what would the end user api look like? would the icon set package(s) depend on the core package or did you have something else in mind?

oblador commented 6 years ago

@MoOx Even with the compatibility package that I do intend to keep for a considerable time, the references in Xcode would break (android should be fine) unless we bundle the fonts with that one. Ideally the compatibility package would just reference the other packages and not contain anything in itself.

@brentvatne: Yeah separate packages that depend on a core package. End user API wouldn't change beyond the import. From react-native-vector-icons/Ionicons to @react-native-vector-icons/Ionicons or similar. That way you could update the core package for RN compatibility but keep the same version of the icon. The public core API is already very small, just simply createIconSet(glyphMap, fontFamily[, fontFile]) and has been stable for a very long time now.

I think we could probably support complex icon sets like Font Awesome in a better way without bloating the core too.

brentvatne commented 6 years ago

would the dependency on the core be a peerDependency? it seems like a good use case for it

oblador commented 6 years ago

would the dependency on the core be a peerDependency? it seems like a good use case for it

Yep!

Do you see how this impacts your expo compat lib? Would make it easier even?

brentvatne commented 6 years ago

I think it doesn't make a big difference either way because we remap react-native-vector-icons to @expo/vector-icons in a babel plugin :) I was just curious because I have some experience with a similar type of structure in react-navigation where we have pulled various navigators out of the core and made them have a peer dependency on the core, which has worked well. I think this is also especially important when using native modules, for example we also recently introduced a peer dependency on react-native-screens. If we had made it a dependency then it's possible that somewhere there may be some incompatible version ranges and we'd end up with two copies of the package in our node_modules, then the app would crash when we try to requireNativeComponent on the same native component twice.

oblador commented 6 years ago

Cool yeah, but I'm thinking you'd probably just need to remap the core package in this case. That way you don't need to maintain the the icon set packages.

brentvatne commented 6 years ago

yup! looking forward to this change

hampustagerud commented 6 years ago

I think we could probably support complex icon sets like Font Awesome in a better way without bloating the core too.

It would be beneficial to be able to opt-out of native code like Font Awesome 5 introduced. At the same time there has been requests for adding fonts that uses different styles (for example #723 and #793). I don't know how Material implements styles compared to Font Awesome but I see that Ant Design also has multiple styles. Maybe this should be a feature supported in the core, somehow, instead of a font-specific implementation or maybe it could be another package which implements it and can be used as an dependency. Ideally, this should boil down to selecting a correct font weight (solid should be heavier than light for example) but that is currently not the case, at least for Font Awesome 5.

oblador commented 6 years ago

@hampustagerud I agree, however it feels like the implementation details differ a lot between the different icons. One way would be to always produce our own font but that has issues in itself. Do you have an idea of the different approaches used and if we could have a unified way somehow?

hampustagerud commented 6 years ago

Not currently but I can look into it (a bit hectic at work at the moment though), for now it would be fine to have the FA5 fixes in a separate package šŸ™‚

daviscabral commented 6 years ago

This makes a lot of sense and I am already getting the advantages from it on react-navigation and other projects that started with this approach.

Looking forward for this new structure - possibly my first opportunity to contribute here too, with docs, code or with helping on issues.

oblador commented 6 years ago

@daviscabral We would love any kind of help really šŸ‘ I realize it's a pretty big ask/issue, but would you want to give a stab at this (with our help of course)?

daviscabral commented 6 years ago

Sure thing - that would be great.

~Is ok to start a PR with packages inside this repo - in a similar "monorepo" style?~

EDIT: Please, ignore the question - I got confused when read about react-navigation - and imagined moving it into several different repositories. :-)

oblador commented 5 years ago

@daviscabral Any progress so far?

daviscabral commented 5 years ago

hi there - sorry for the lack of updates here. I have started something, but had to stop because of an international work trip. I'll push a PR for appreciation as soon as I get some free time this weekend.

eddskt commented 4 years ago

Any updates about Enable themes (e.g. "outlined") of MaterialIcons?

iljinjung commented 3 years ago

I want to use the themes!! šŸ„²

johnf commented 6 months ago

Significant work has been completed on moving to a new monorepo structure. We would love feedback, please see #1612