-
-
Notifications
You must be signed in to change notification settings - Fork 85
Import woodoo #8
Comments
Thanks for taking the time to check it out. 😄 Currently there is not a way to use both. ReactN exports a copy of whatever version of React you have installed. It couldn't know to look for a ReactM package. As a developer, what code would you want to have to type to merge ReactN with another React clone? Off the top of my head, a setPackage helper function that defaults to React if not called with a different argument like ReactM. Alternatively a REACTN_PACKAGE environment variable that defaults to |
No probs. I was actually thinking about this a lot. Thinking about your motivation to do it this way, also reusability and the design patterns. Now I don't want to be internet troll, I rly though about this, and even though on some level, I like this. Eventually as the idea in my head matured, I don't think it's a good approach. First of all, you are reexporting a foreign package, so there's no clear way to document the api as you don't rly know what you are exporting. Secondly, it can be confusing for people, as there is no way where you changed something and which are original components. Lastly, the aforementioned composition of multiple libraries would be a pain. In general, the good solution to problems are the ones that can be easily understood and reused. If for example all libs started using this approach, imho things would get quite confusing. Boiling down to the core of why this lib was created (and taking into consideration that it's quite new), I am starting to think that it might be best to ditch the idea of modifying classes. Instead this could be very elegantly implemented as custom hooks. Yeah I know it's not out yet in stable version, but looking into it, it seems like it was destined for this lib. The thing is, the component class doesn't rly have to be different to handle globalState, does it? At the end of the day, it's still just a view layer. If we start going the way of adding stuff to base classes, we might end up inflating the whole thing, hurting performance and so on and so on. So to sum up, I clearly see why you did this. The wrapper hell is a real thing in react these days, but it seems hooks are the way to solve this. I am not saying I am providing any real solutions, just giving my 2 cents :) Hope you find it at least a little bit useful. |
The trade off in architecture for less boilerplate is well established. There's no denying it. That is precisely why the target project for this package is small or personal. This package introduces limitations in extensibility, such as migrating away from React and hard coupling. The point of this limitations isn't because they are better per se, but because the benefits they provide to small projects outweighs the benefits of using extensible alternatives. That's just a fact of the matter, and I don't think there is a way around it. If you don't want boilerplate, you have to lose extensibility and best practices. Sometimes that's a good trade to make. This package is not for every occasion: it is for occasions where that trade off is worth it. Custom hooks are already implemented in this package, via You also do not have to modify the definition of React component to use this. The Maybe that's really all that matters? I can't expect to import from ReactN and use both ReactN and ReactM, but I can import the decorator from N and decorate M with it. I think it may be a better solution to simply rely on the HOC or decorator than a helper function for changing the root package from React to something else. What do you think? Is that a "solution enough"? The more I think about it after having said it out loud, the less sense it makes to expect M features when importing only N. The N decorator will add N's features to any component, even M's. The N HOC will support any component, even one that conflicts with N's implementation of component, meaning you don't have to use N's implementation of component at all. This issue may already be resolved with these two methodologies. |
So, most of the issues with typescript getting this to work well relate to the hijacking of react and the need to then import all other objects types straight from react. So, the HOC idea works for me, but.. is there a way to get access to setGlobal using just the HOC without decorators or replacing component? |
Does the I'm not opposed to having it be built into withGlobal, though. I simply didn't think of it at the time, but it should probably be a different Issue. |
Makes a lot of sense what you said. I think reasoning about problems like this quite complicated and gets complex very quickly. The "real" problem here definitely is being able to combine with hypothetical reactm. Helper function could solve that imho. Also good news this supports hooks, like I've said, haven't rly tried it properly yet. I am looking forward to see where this goes. Imho it can definitely power some small hobby projects or maybe some component previews like in codepen and what have you. I think I am happy with what was discussed here. Leaving it to you to close it if you agree ;) Thanks! |
Awesome. It was good food for thought. I don't think it makes sense to import only N and get both N and M included. One would need to import both, in which case I think the decorator is the ideal solution for that use case, and it's already supported. |
Haven't played with this yet but it definitely seems promising. One thing I either misunderstood or might be wrong design is the way you import these components. While just replacing 'react' with 'reactn' sounds rly cool and easy to use. What if somebody in the future releases a lib called e.g. 'reactm' also adding some functionality. What would you do to use both at the same time? I know there are some HOCs you could use, but this still feels a little shady to me.
The text was updated successfully, but these errors were encountered: