microsoft / FluidFramework

Library for building distributed, real-time collaborative web applications
https://fluidframework.com
MIT License
4.72k stars 532 forks source link

Design for multi-view support #1042

Closed ChumpChief closed 4 years ago

ChumpChief commented 4 years ago

Over the last few weeks I’ve come to the opinion that we should deprecate IComponentHTMLView and IComponentHTMLVisual (and more importantly the addView() and remove() methods they hold). This design issue lays out my reasoning and presents some view of what this direction might look like.

Current state

The runtime team has fielded numerous questions and confusions from our partner teams on how these interfaces should correctly be used, what guarantees exist, who manages view lifetimes, what is “scope” in addView() for, etc. We don’t have great answers to these not just because they are challenging questions, but because we are internally inconsistent in our own usage in our own repo. For example:

Problem framing

The addView() API gently implies that there is some level of separation between a model component and view component, since it seems it should generate a one-many relationship. However this API is generally made available on the corresponding model component – which has several implications:

  1. It enshrines a single view component as preferred for that model component. Providing multiple view component options for a single model component type must be solved by some other contract outside this API.
  2. It prevents that model component from being ignorant of view components that it may be fed into. It is not conducive to developing a purely model component.
  3. It assumes that the model which addView() is called on will be the same one attached to the generated view component, and that it will be the only model attached to that view component. It does not lend itself to building views that compose data from multiple models, or views that can be initialized with no backing model initially.
  4. It encourages tight coupling between model and view, rather than establishing an open API surface on the model component that other views might plug into (e.g. the MathInstance calling remoteEdit() on each of its views).
  5. It requires the host to have intimate knowledge of the view being created, in order to pass the correct scope argument when calling addView(). Anonymously creating views cannot be done confidently.

How do we retain the functionality if we deprecate the API?

The main piece of functionality that addView() intends to add is the ability to have multiple views on a single model. But this could also be achieved by having the view component stand alone, and accept (through a contract of its choosing) the model that it should bind to. I take this approach in draft PR #767 for the Todo component, which introduces no new interfaces. The end result resolves many of the problems above:

  1. A single model can be attached to multiple different view component types, by simply creating a NewTodoView or NewTodoItemView and passing the model in to those views.
  2. The model is completely ignorant of the views.
  3. A view can be created with any model. It would be easy to create the view with no model (and add some default rendering), or even rebind to a different model after creation.
  4. Building it this way forced me to think through the API surface for the model components, scoping its public API surface and adding eventing that the views can listen to.
  5. In this current PR, the host must still have intimate knowledge of the view being created, in order to successfully pair it with a model. This is probably best solved by adding a set of discovery APIs that can help views and models find one another, but another variant could have the view create/own its own model upon its creation.

How does this help resolve the ambiguities our partner teams have encountered?

By removing these methods, we generally shift these concerns to be external to Fluid. They should be solved by using existing patterns in their respective frameworks. E.g. React-based views will use componentDidMount, componentWillUnmount for lifetime management, there is no view collection to be managed, scope is reframed as properties, etc.

What’s next if we go this route?

If we choose to deprecate these interfaces, I think our path would be:

  1. Pick an exemplar component to perform this view/model separation on, converting it from addView() to using render() only. This will serve both as an example to share but also as a learning experience on what the conversion looks like.
  2. Investigate the Bohemia repo in collaboration with the Bohemia team to validate this approach carries over cleanly there.
  3. Mark the interfaces as deprecated, point partner teams at the component from (1) as the direction to move in.
  4. Complete elimination of usage of these interfaces from the Fluid repo by converting the remaining components, loader, etc.
  5. In a future release, remove the interfaces.
christiango commented 4 years ago

Not sure I follow how you will defer to the framework for things like unmount. For example, consider this scenario:

You click on a new page in the Fluid Preview left nav to navigate to a new container. The current container is removed from the DOM. The component rendered from the current container would like to be notified by the host when it is removed from the DOM so clean up like the removal of scroll handlers can be done.

ChumpChief commented 4 years ago

In general, I'm proposing that Fluid should not introduce any new concepts into solving this problem, which already exists and has solutions (i.e. there are already advanced apps out there that navigate between views and must manage lifecycle of those views). In React, you're probably using componentWillUnmount to clean up handlers, in Vue probably beforeDestroy, Angular ngOnDestroy.

In general we're trying to avoid building a competitor to these frameworks, and rather get out of the way of developers who are using them.

christiango commented 4 years ago

That makes sense, but do these handlers get called automatically when a dom node is destroyed by the host?

What pattern do we want to suggest for components, like Scriptor, that don't use a framework. Do mutation observers address this need?

ChumpChief commented 4 years ago

That makes sense, but do these handlers get called automatically when a dom node is destroyed by the host?

No - but in general, direct DOM modification outside of what is provided by the framework is likely to fail in various ways and probably shouldn't be done - e.g. similarly if you directly reparent nodes, insert unexpected nodes, etc. then things probably break. I expect view.remove() would have the same problems if you chose to destroy the DOM node without calling it.

There may be a framework that does scrutinize DOM modifications via mutation observers, but I'm not aware of such. I'm not sure what the performance implications would be at scale.

What pattern do we want to suggest for components, like Scriptor, that don't use a framework. Do mutation observers address this need?

If you're rolling your own solution then you'll probably need to roll your own lifecycle management as well - I expect you'd want something very similar to the major frameworks.

DLehenbauer commented 4 years ago

You can interpose a custom element to get lifecycle events.

christiango commented 4 years ago

So the host is the one that removes the dom elements for things like a route change, not sure I follow how we expect that to be communicated to the component then without a veiw.remove() like contract.

christiango commented 4 years ago

(And I have similar concerns about mutation observers, but have not investigated it myself)

ChumpChief commented 4 years ago

So the host is the one that removes the dom elements for things like a route change, not sure I follow how we expect that to be communicated to the component then without a veiw.remove() like contract.

Our current visual APIs (render/addView/remove) combined with usage patterns like the host performing the DOM removal are effectively building a new UI framework library (i.e. a React competitor) -- this contract you're describing is very analogous to ReactDOM.unmountComponentAtNode() plus componentWillUnmount handler.

With this proposal, the runtime would become less-opinionated about UI framework and contracts, leaving it up to apps/components to bring their own framework -- be that React, Vue, or one of their own creation. You would be free to establish any contract you would like.

Then, I think the main remaining problem is how to ensure interop between apps/components of differing frameworks, and/or interop between apps/components that are unaware of what each others' frameworks are (which I've been calling "anonymous" components). For this, I have some very early thoughts (#1043) on adapters for this purpose. But this would be at the framework level, external to the runtime and totally optional.

christiango commented 4 years ago

I think we definitely need a well maintained abstraction layer here for the component ecosystem to work, otherwise the overhead of working with Fluid will limit it's adoption (would put a ton of work on both hosts and component authors). I'm a bit concerned with making this large of a change to the component model at this point in the lifecycle. Maybe once I see this working in Bohemia I'll be a bit more receptive :)

kurtb commented 4 years ago

The scope field is incredibly important. It's too bad it has caused such confusion. That's likely on me for not having good enough examples.

The early component interfaces had an explicit register method to do bi-directional component exchange between the parent/child. This lets the child query for capabilities of the parent and vice versa. Most cases today tend to be unidirectional since it just ends up being div hosting. But scenarios where the component ends up in a native app and the native app has the ability to render it are a case for querying the parent. Or the example we have of the math component registering commands on the UI of the parent.

It also lets the parent (teams, scriptor, tablero, owa, flow view) restrict what a child has access to. The scope is how host services should be passed to the child (UX command access, clipboard, etc...). This is especially important in a zero trust mode.

Rather than one blanket register call the design moved to having a scope passed on a per scenario basis. The addView is rendering specific. Intelligent services, etc... will have the same pattern. But it's important for the thing being made use of to have access to the thing using it.

A framework may want to split model/view but at the platform level we can't assume hosts have any idea on how to find views for a component. It's important for copy/paste across host interfaces that the ability to render an arbitrary component - fetched form a URL - be preserved.

I can see how the current render vs. addView/removeView can cause confusion. The split was setup to have a dead simple way for new authors to make something that draws quick (render). But then expand to the more complex part of dealing with scope and having a component that can have multiple independent views on the same page.

I'm curious to see what these new interfaces look like - but the above two requirements are important to preserve.

DLehenbauer commented 4 years ago

I think the main remaining problem is how to ensure interop between apps/components of differing frameworks

I thought the effort to standardize on WebComponent custom elements for framework interop looked promising.

christiango commented 4 years ago

I think the main remaining problem is how to ensure interop between apps/components of differing frameworks

I thought the effort to standardize on WebComponent custom elements for framework interop looked promising.

Have they sorted out the issue with React synthetic events vs native DOM events?

DLehenbauer commented 4 years ago

There is a reasonable story. You inject a shadow root at the component boundary, which causes React to move it's DOM <-> synthetic conversion down to the shadow root's doc fragment. (+@barnawhi who researched this.)

ChumpChief commented 4 years ago

1043 would be a great place to capture thoughts on the framework interop topic!

ChumpChief commented 4 years ago

I'm going to go ahead and resolve this issue. The design above around separation of model and view is the direction we're taking in our repo and examples (dice-roller, todo, etc.), and is what we're promoting publicly. A good chunk of the work is already done as well (view adapters, mountable views, and removing usage of addView() from the repo).

There is remaining work to better support specific patterns like Spaces (#2352), and to deprecate the view interfaces (#1041) that will be tracked with their respective issues, but the overall design is fairly solid at this point.