search

artsy / reaction

Artsy's publishing components
MIT License
356 stars 74 forks source link

[RFC] Split reaction into multiple bundles #1685

Closed zephraph closed 5 years ago

zephraph commented 5 years ago

Reaction is a labor of love that every team touches in some way. It's growth has been both rapid and (at times) a bit chaotic. I think it's fair to say it's grown beyond it's original intent at this point.

There are at least two major categories of things that live inside of reaction. Apps which are full experiences that are injected into force and components which are imported and injected into other projects. Consumers like volt and positron will never use the apps that are consumed by force. Breaking some of these pieces down into separate packages could mean pulling less transient dependencies into downstream packages. Another advantage would be reducing package churn on unused areas of code. For example, positron wouldn't need to upgrade reaction if the artwork app changed. Of course, nothing comes without downsides. Generally this would mean supporting a monorepo structure or some other mechanism of separation that will come with its own challenges. With the monorepo approach I'm particularly concerned about any increased difficulty with linking across projects.

Regardless of the approach, I still think we should pursue breaking reaction down into smaller, more maintainable packages. We'll need to decide what granularity of packages makes sense too. Should every app be its own packages? Should there be a components package, or should certain component groups (like publishing) be pulled into their own package?

Thoughts or questions?

orta commented 5 years ago

With the monorepo approach I'm particularly concerned about any increased difficulty with linking across projects.

This is what yarn workspaces are good at FWIW

ashfurrow commented 5 years ago

we should pursue breaking reaction down into smaller, more maintainable packages

I'm a big 👍 on this, but as you point out it's easier said than done. My only concern is a combinatorial explosion of packages and an increasing amount of time spent wrangling our various packages.

One thing to keep in mind as we move forward on this is of course our Engineering Guiding Principles. The one that comes to mind is "incremental revolution" – our solution to splitting out Reaction should be bold, but would ideally shield most frontend engineers from dealing with additional complexity in day-to-day product work.

williardx commented 5 years ago

Could we just fold the Force-specific apps back into Force?

zephraph commented 5 years ago

Could we just fold the Force-specific apps back into Force?

That's certainly a possibility, but that wouldn't necessarily fully eliminate the need to break down reaction. Still, I think it's a valid point and something worth considering.

damassi commented 5 years ago

Not suggesting that we move apps back into Force, but worth noting that we now have full Relay compiler support should we need it, as well as TypeScript and Jest. A lot of the initial momentum for Reaction was around building a modern / safe dev environment and since then Force has been upgraded to support all of those features.

That said, the thought of folding stuff back in is indeed intriguing. We would need to move our router code into its own package (something I've been wanting to do for a while) and the testing tools, too. (There are a lot of things in Reaction that are now ready to be separated out, including individual apps.) On the Force side we would need to add Storybooks but that seems simple enough.

Folding stuff back in would help us get to full-page React in a safer way too, and allow us to factor out Stitch.

zephraph commented 5 years ago

The more I think about it, the more I like the idea. Moving the apps into force greatly reduces the need to link. Being as that's a pretty continual source of issues that'd make development easier.

damassi commented 5 years ago

Any thoughts @alloy?

alloy commented 5 years ago

I think moving apps into Force makes sense, from a conceptual perspective at least. Personally I’m a little hesitant about developing inside Force with the overall setup being slower than working in an isolated environment, but if most people work on Reaction from inside Force already anyways then that point is very moot.

@zephraph One thing to keep in mind is that if we’re ever to move to a MP v2 schema setup, the individual apps would still need to have their own relay-compiler setup so they can use different schemas. That’s not a reason to not do this move now, but it may be a good reason to still have a workspace setup.

zephraph commented 5 years ago

Would an omakase shared setup be able to help resolve that?

damassi commented 5 years ago

Personally I’m a little hesitant about developing inside Force with the overall setup being slower than working in an isolated environment

I don't think this move should exclude a storybooks setup; that abstraction seems to have worked well so far!

alloy commented 5 years ago

@zephraph Not sure how. Can you elaborate?

damassi commented 5 years ago

One thing I want to raise re moving apps back into force (separate from splitting Reaction apart) -- we should be sure that we're not coming up with solutions to problems that aren't really problems. Linking between Reaction and Force currently works well and most of the rough edges have been ironed out over the past year.

(I do think there's merit to this idea but I don't think it relates to existing toolchain issues; if we decide to migrate our view needs to be a bit wider and really dive into the pros / cons.)

orta commented 5 years ago

If we move the server + routing into reaction (ideally via sub-packages) then it would be pretty easy to add visual regression testing to new apps

orta commented 5 years ago

Re: Moving apps back to force.

I think one of the goals for reaction should be to provide a reliable dev env for working in web apps too, not just as a component library. For example, teams work entirely in Emission for months without working in Eigen because they trust that it will work the same across those two environments - for dev Emission provides a library host and provides a component library. It took a bunch of work to get that level of trust in Emission.

Ideally, that work should happen in Reaction too. Right now though, Reaction does the component library well, but doesn't provide a useful enough host. To do so has two main angles:

So, IMO, moving apps to live in force is basically giving up on having a solid reliable single place for doing your work and leaves a tonne of complexity in having to have force set up, linked and handle partial aspects of the rendering process.

Ideally all it would look like in force to use a reaction app, is to pass it a single express function and that's all the integration is (which emulates how we pass a single native screen across the Eigen/Emission gap)

alloy commented 5 years ago

Ideally all it would look like in force to use a reaction app, is to pass it a single express function and that's all the integration is (which emulates how we pass a single native screen across the Eigen/Emission gap)

:ceiling-cat: Just want to note that that is what this PR was working towards (although some stitching is still necessary for things like headers or modals that still live in Force).

orta commented 5 years ago

Hah, I swear you and I are a semi-hivemind @alloy

damassi commented 5 years ago

Emission provides a library host and provides a component library. It took a bunch of work to get that level of trust in Emission

I don't think Reaction, Emission, Force and Eigen should be compared since native is inherently more complex, is C-based, etc. The only practical similarity is React and Eigen being a host environment. However, Eigen as a development environment is much different than Force as a development environment, and this needs to be fully appreciated.

When thinking in terms of Reaction and Force and their overall infrastructural setup, things are nearly identical. Both use the same babel config, and roughly the same Webpack config. When Force boots, it iterates over files located in an assets folder and creates separate bundle for each asset. Once things are booted, compiles are cached according to asset, and compile times are between .5 to 1.5 seconds -- fast. Working inside of Force requires no special infrastructure (except for stitch) and is pretty simple. Adding things is as easy as adding a new asset file, which Webpack automatically picks up. Additionally, if we wanted to detach Force from the complete asset pipeline that runs artsy.net (to run, say, storybooks, or whatever else), we could pass an array of Webpack configs into the webpack-cli directly in force. Separate processes, without the boundaries of package management, linking, or special infrastructure, allowing for a pretty seamless iterative-evolution process.

By moving all of this into Reaction and creating reverse-bindings between Force and Reaction via stitch, we're a) increasing our reliance on a tool that was meant to be used temporarily; and b) creating a maintenance burden for reasons that might not be necessary.

One of the virtues of the force / Ezel architecture is that it's extremely flexible and allows for change over time. We're currently onboard the Reaction / Relay / React train right now, but for how long? The web is constantly evolving, which is also something different from Native in that the iOS platform is fairly locked down.

Currently, there are few problems and low complexity when communicating between Force and Reaction -- all that we need is an event bus, Mediator, which is passed in as a context when we instantiate a route. When we need to call old code, we dispatch an event. It might not be fancy, but its also not causing any problems and is easily understood by developers of all skill levels.

All of this is not to say that breaking apart reaction (the original aim for this RFC) isn't a good idea, but that we shouldn't create more complex infrastructural communication layers between Force in Reaction as described in https://github.com/artsy/reaction/pull/1524.

I think one of the goals for reaction should be to provide a reliable dev env for working in web apps too, not just as a component library.

This is already exactly what we're doing, thanks to GraphQL and Relay's network layer. Multiple apps have been built out quite extensively in Reaction, and during the later stages tested in Force. This is how /artist, /artwork, /order, /collect and /collections were built. Reaction is already far past a component library, and has been so since we started using the new routing setup (which Reaction controls with the exception of some Force tech-debt from when we first laid down the pattern (this code should be controlled by Found, in Reaction)).

Using the new Artwork page as an example, aside from triggering the login modal and mounting our router, there is a one-line dependency from Force; Reaction is able to handle everything else. And to trigger the modal, it's mediator.trigger('login', { ... }). It's not a far stretch to imagine a session being updated via a mutation, in which case we can call the Login modal directly from Reaction and skip the mediator completely. Once the top nav bar is rebuilt in React there will be almost no connection to Force from the perspective of new apps, and rarely do we need to call out to old Force code (with the exception of modals, which are increasingly being built in Reaction).

mzikherman commented 5 years ago

Coming at this a bit late, I'm still trying to understand the problem/issues currently facing Reaction. I totally understand that having a component lib and full(er) apps living in the same repo feels odd, but we've ironed out all those kinks as @damassi points out.

yarn link and the general state of front-end web development seems really...awesome these days. I'm seeing two issues mentioned in the top comment as possible problems:

However, I also thought that one goal we had was to be able to rebuild the full pages in Force (including the NAV), inside of Reaction, and begin chipping away at the bundle sizes in Force.

Moving full apps from Reaction into Force feels very murky and as @orta points out I think will really increase complexity.

Now, if people are fine with components and apps living side by side, but still want to explore better organization and handling of dependencies (via splitting Reaction or a different repo setup), I think that would be more worthwhile than moving apps into Force. We should be careful however, even in just splitting up in Reaction, that we're not adding a lot of complexity and intermediate states and a lot of time/effort to reach an ideal, vs the current agreeable state. Personally, Reaction doesn't at all feel big enough that it's hard to work with, whether it's storybooks, a linked state, an app component, etc.

zephraph commented 5 years ago

The original intent of this RFC isn't about moving things back into force, but about restructuring reaction's repo in a way that allows us to split it down into multiple packages. I think once we go through the exercise of reorganizing reaction in its current state that'll give us the freedom and flexability to have future conversations about where things may need to go.

damassi commented 5 years ago

As an aside, we're getting pretty close to completely decoupling Palette from Reaction. What remains at this point in terms of a potential monorepo split would be:

sweir27 commented 5 years ago

I'm not opposed to this generally! But having worked only minimally in a monorepo/etc. I'm curious if there's a way to do this in an incremental way and to evaluate whether it's what we want to be doing before we make a big change.

Some thoughts re: the discussion of moving things back into force (or in general re-thinking how we architect things):

I know this ☝️ is a separate discussion than the original RFC, but if it informs this change in any way it seems worth mentioning!

zephraph commented 5 years ago

There's a lot of good discussion here, but I haven't had the time to dedicate to resolving this RFC. I'm going to close it for now, and we can re-open it once we have more time to work towards a solution.