Polaris-react: Should App Bridge be removed from Polaris?

[vividviolet]

Issue summary

Let's discuss the pros and cons of removing App Bridge from Polaris

Related discussion: https://github.com/Shopify/polaris-react/pull/695#issuecomment-447074291

@tmlayton: Personally, I’m leaning heavily towards removing app bridge from Polaris entirely. It seems to have caused more issues and confusion than it has been helpful. I’m thinking we would still provide a set of React components which wrap app bridge (maybe even keep the same names), as well as an AppBridgeProvider for instantiation. Wherever these components live, we can move the ShopifyRoutePropagator there as well.

[probot-shopify[bot]]

👋 Thanks for opening your first issue. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines. You can also join #polaris on the Shopify Partners Slack.

[vividviolet]

@tmlayton I think a lot of work has already been done to integrate App Bridge with Polaris. The documentation does outline what's expected when using Polaris components inside an embedded app. There is definitely room to improve the docs. The nice thing about having the App Bridge integrated with Polaris is that developers can use the components as usual without thinking about App Bridge aside from the initial AppProvider set up. I think we need to work toward clarifying the docs and resolving the remaining bugs instead of removing App Bridge from Polaris entirely.

I think separating App Bridge out of Polaris would have the following drawbacks:

• inconsistent user experience between an embedded app using normal Polaris components vs. apps using the Polaris wrapped App Bridge components. For example, a lot of work has been put into standardizing the app title bar so that it is consistent and accounts for new features such as secondary nav. @katbuilt might have more thoughts on this
• inconsistent user experience when we allow an app to use a mix of both normal and App Bridge components
• developers being confused as to when to use the Polaris wrapped App Bridge components vs. the regular ones. For example, they might not know to use the App Bridge modal which results in the app displaying a modal that does not cover the full Admin and other UI differences

[tmlayton]

Pros:

• Clear separation of concerns – Polaris is for its owned UI and not delegation to other APIs and should have little concern with window
• Avoid stand-alone apps having an increased bundle size or hack a workaround to remove the app bridge module
• Reduce support load in Polaris React for app bridge and embedded app issues which are not related to Polaris React
• Reduce code complexity
• Allows apps to choose to use Page component in multiple ways
• Give consumers more control of how/where they instantiate app bridge
• Avoid Polaris React having to worry about keeping the app bridge version up-to-date
• Would provide a place to create other React-wrapped app bridge components like ShopifyRoutePropagator

Cons:

• Have to import another library for UI stuff

I don't think the confusion of which component to use would be much of a problem. Modal, Loading, and Toast do not work if there is no Frame. So we can suggest, if no Frame is present in their app, they use the embedded version of Modal, Loading, or Toast if they are trying to use the one in Polaris React. The only two components left are Page and ResourcePicker, and ResourcePicker is an embedded-only component already. So it is just Page which could be possibly used in both ways, but as #695 suggests, there is a need to do so and is already being worked around.

The upgrade path would be fairly simple if we moved React-wrapped app bridge components to their own library:

- import {AppProvider, Page, Modal, Toast, ResourcePicker, Loading} from `@shopify/polaris`;
+ import {AppProvider, Page} from `@shopify/polaris`;
+ import {AppBridgeProvider, TitleBar, Modal, Toast, ResourcePicker, Loading} from `@shopify/polaris-embedded`;

function App {
  return (
-    <AppProvider shopOrigin="foo.myshopify.com" apiKey="bar">
+    <AppProvider>
+      <AppBridgeProvider shopOrigin="foo.myshopify.com" apiKey="bar">
+        <TitleBar ... />
         <Page ...>
           <Modal ... />
           <Toast ... />
           <ResourcePicker ... />
           <Loading ... />
        </Page>
+      </AppBridgeProvider>   
    </AppProvider>
  )
}

I think at this point we would also, as the code above suggests, diverge on the name Page, having the React-wrapped app bridge component be called <TitleBar /> to match the action name.

[sdn90]

I think extracting the embedded functionality out of the UI components might be a good idea. I've been using the library since it was released to the public and most of the problems I've run into were related to embedded apps. If it was separated, I'd imagine it would be easier to reason about and documentation would be easier to write and read.

I feel like people are going to be confused on how these components work no matter how the integration is implemented since the general concept of embedding an app is not common outside of the Shopify ecosystem.

[keyfer]

Based off of my, albeit somewhat limited, experience working on Polaris components wrapped around AppBridge, there is an argument to be made for the level of difficulty involved in developing and testing these components in isolation. Unless you have an existing embedded app which you could work on, setting one up to create the union between AppBridge and Polaris just to develop/test functionality of these components could be tedious.

I also wanted to second @tmlayton's point regarding the separation of concerns. I understand the argument for keeping things the way they are, as there might be confusion between two "component libraries", but I think this is a worthwhile trade for having specified instantiation of AppBridge, a Polaris with little regard for window, and two separate packages which can be highly aligned and loosely coupled.

[gf3]

Based off of my, albeit somewhat limited, experience working on Polaris components wrapped around AppBridge, there is an argument to be made for the level of difficulty involved in developing and testing these components in isolation. Unless you have an existing embedded app which you could work on, setting one up to create the union between AppBridge and Polaris just to develop/test functionality of these components could be tedious.

that's probably a good indicator of how much more difficult it is for partners, and probably a pain point we should focus on optimizing

I also wanted to second @tmlayton's point regarding the separation of concerns. I understand the argument for keeping things the way they are, as there might be confusion between two "component libraries", but I think this is a worthwhile trade for having specified instantiation of AppBridge, a Polaris with little regard for window, and two separate packages which can be highly aligned and loosely coupled.

i don't feel too strongly about either direction, technically. my main concern is supporting existing developers. will this make it more difficult to build and support embedded apps? will this make embedded app developers unable to upgrade to the latest polaris?

instead of pushing complexity out to developers, i strongly feel that we should take on the burden of solving complex problems where necessary—i'm wondering if this is one of those times?

[danrosenthal]

Proposed next steps

I feel pretty strongly that the next step should be to remove AppBridge from Polaris following the library's deprecation strategy, while providing a sensible upgrade path for current users. That upgrade path would be a public package in the Polaris nebula, supported directly by the App Bridge team, as they have the most context on this project and are closest to it. That package would contain a provider component (using the createContext API) that instantiates AppBridge, as well as the following components, which would be wrapped implementations of their Polaris counterparts which delegate rendering to AppBridge and only expose the API that is supported by AppBridge:

• AppBridgeToast
• AppBridgeLoading
• AppBridgeModal
• AppBridgeResourcePicker

It would also contain an AppBridgeTitleBar component that is an implementation of the embedded notion of a title bar which also delegates rendering to AppBridge. Polaris could then provide an option in the Page component to not render its own title bar, allowing both to exist harmoniously side by side.

Rationale

I won't spend time re-hashing the arguments in favor of removal that have already made on this issue. I agree with each of the points Tim made, and want to emphasize the support problem. Currently about 10% of the open issues in this library are related to the App Bridge in some way. There also doesn't seem to be much indication of that amount of support debt slowing, even following significant improvements to documentation.

The above solution would result in a significant reduction in Polaris support debt, separate concerns in a meaningful way, reduce package size for the majority of Polaris consumers, allow more focused, clearer documentation, and create a smaller feedback loop for support. The only drawback I can see is the separate package problem, but I don't see that as being significant so long as we communicate the change clearly as part of deprecation and removal.

[lemonmade]

From my perspective, drawing a hard line between these two is not a good trade of ergonomics for the developer versus ergonomics for Shopify (the library author). One of the benefits we see in Polaris is that we can eventually change how any given component is implemented without meaningfully disrupting consumers (many of the restrictions are in place exclusive to optimize for this). Saying that we have an AppBridgeToast or whatever else basically hardcodes our implementation of those components; if we ever decide that some of these components would be better rendered in the iframe, we are completely stuck (setting aside that, in an ideal future, we find a way to make it so that there is no iframe at all.

Some time ago, I proposed to Tim and Dom the notion of an adapter for Polaris that the library could ask to determine how some components would be handled. The adapter could replace or augment some components; this enables things like App Bridge, but also makes it easier for Shopify Web to do things like intercepting all page titles/ actions and sending them to the mobile app when we detect we are in a web view.

This has the gist of it: https://gist.github.com/lemonmade/f3e0effffaa1068df18400e1777f2305. I understand that it might not solve the "people think an app bridge issue is a Polaris issue" problem, but people have posted issues that are actually problems in Web or some other random part of the system, so I think that's mostly unavoidable. This pattern does at least create two distinct libraries, and I think the messaging can be more clear as a result.

[nwtn]

There are lots of great points to consider in this issue. Since it involves multiple teams, I suggest we meet in real life to discuss. I’ll set something up, and we can update this thread after our discussion.

Edit: to Tim’s point below, just because we’re meeting doesn’t mean we have to stop discussing here. All of this is great input for us to take into that in-person meeting. :)

[tmlayton]

Lots of great questions. Meeting to discuss would be good, but I think it is also worth continuing to discuss in this thread. The downside to a meeting is we then have to proxy a voice for Shopify Partners and I’m curious to get more of their input on this.

At the core of the points above is this notion removing app bridge is worse for app devs (more work/complexity) and gives us less control – and I believe it is better to for an app developer to import the embedded components separately. It is a clear mental model, there is no overhead of what props work in which context, debugging is easier, and finding support is easier. I think there would be no change in the amount of implementation control we have, it is just in two places. The example below about AppBridgeToast is a theoretical problem we don’t have and I don’t suspect we will. But hey, maybe I’m wrong.

While there are things which benefit us as the library authors, if removing app bridge would ultimately make things worse for app devs, then I’m not interested in it.

In the meantime, I’d like to address a few of the questions directly.

---

will this make it more difficult to build embedded apps?

Maybe, but also might make it easier to debug and build. It clarifies the API, mental model, and where to go for support.

Will this make embedded app developers unable to upgrade to the latest polaris?

Same as any major version upgrade – it would actually be less work than upgrading to v3.0.

instead of pushing complexity out to developers, i strongly feel that we should take on the burden of solving complex problems where necessary—i'm wondering if this is one of those times?

I would not qualify this as pushing complexity to Shopify developers but rather sharing an important part of the mental model and relinquishing control app devs need. We have had feedback both internally and externally the current state is confusing, hard to debug, and it's frustrating for app devs to get support. I think this is a symptom of trying to force two things together and obfuscate the line.

drawing a hard line between these two is not a good trade of ergonomics for the developer versus ergonomics for Shopify (the library author)

We already drew a line between the two up until v3.0 with the embedded bundle and tried to remove the line in v3.0 by deleting the embedded bundle. From my perspective (mostly based on the feedback I mentioned in the above question), removing the line has been a net-negative.

Saying that we have an AppBridgeToast or whatever else basically hardcodes our implementation of those components; if we ever decide that some of these components would be better rendered in the iframe, we are completely stuck

And this didn’t seem to be an issue before – we similarly hardcoded our implementation to Modal, Page, and the exposed EASDK methods by importing from @shopify/polaris/embedded and grabbing the methods from context, respectively.

[tmlayton]

We are moving forward with this work, which is tracked in https://github.com/Shopify/app-bridge/issues/734. The initial work to offer a replacement components is here https://github.com/Shopify/app-bridge/pull/928.

The library will be named @shopify/app-bridge-react and each component name remains the same expect for Page, which is being changed to TitleBar.

The update path will look exactly like this:

- import {AppProvider, Page, Modal, Toast, ResourcePicker, Loading} from `@shopify/polaris`;
+ import {AppProvider, Page} from `@shopify/polaris`;
+ import {Provider as AppBridgeProvider, TitleBar, Modal, Toast, ResourcePicker, Loading} from `@shopify/app-bridge-react`;

function App {
  return (
-    <AppProvider shopOrigin="foo.myshopify.com" apiKey="bar">
+    <AppProvider>
+      <AppBridgeProvider shopOrigin="foo.myshopify.com" apiKey="bar">
+        {/* TitleBar can be used along with Page, or as a replacement for Page, up to your app’s need */}
+        <TitleBar ... />
         <Page ...>
           <Modal ... />
           <Toast ... />
           <ResourcePicker ... />
           <Loading ... />
        </Page>
+      </AppBridgeProvider>   
    </AppProvider>
  )
}

Besides for importing the components from a new package and the name change for Page -> TitleBar (this is because they are truly two different components), there are no other breaking changes. The App Bridge related props stay exactly the same, with some no longer required.

Once the first release of @shopify/app-bridge-react is available, we will add a deprecation notice to all areas in Polaris React which delegate to App Bridge, as well as provide upgrade instructions. We are aiming to remove App Bridge from Polaris React entirely as of v5.0.0.