extensibility pattern

[chancancode]

@ef4 @runspired

[ef4]

My summary here would be: override defeats static bundle analysis. So in cases where one really wants to consume an extensible service without sacrificing bundle optimization, some other pattern would be required.

[chancancode]

My understanding of the problem is that an addon wants to provide a well-known service that other addons can depend on and build upon.

The concrete example is Ember Data wants to provide a store service that the application implements (by subclassing the abstract version probably), which guarantees a minimum set of core API once the app fills in the app-specific details. Any addon can use the core API to do stuff in the app. Let's call this first problem the "well-known service" problem.

Second, the additional challenge is to ensure all of these can be lazy loaded on-demand, following the app's actual usage. Let's call this second problem the "module graph optimizability". Personally I consider this a nice-to-have, it is probably not the end of the world if something core to the app like this ended up being forced into the initial payload, but I agree it would be nice to solve this.

Thirdly, the additional complexity is that there are addons that wants to define extensions to these APIs that the app is expected to implement, which then can be further built upon. (?) Let's call this third problem the "extensions pattern".

Finally, it would also be nice if there is some ways to make this play nicely with with TypeScript and get more type safety out of it, rather than relying on "trust me" type casts all over the place.

Let me know if I got that right on not.

As a baseline, I want to establish and get on the same page that this design does not make anything worse than it is today, where "today" means:

1. For the first problem, the coordination happens today over the contract of "this service has the well-known name service:store and it is "guaranteed" to exist at runtime with the right implementation (or rather – if it didn't, then it's okay to just blow up).
2. For the second problem, I am guessing that there is no additional safe guards or guarantees here – it's just handshakes of "if you use this addon you better have these implemented on your store or it won't work", and that the TypeScript situation already requires runtime assertions or unsafe casts.
3. And obviously we don't solve the third problem today and it's always in the initial bundle.

So, as a baseline, I want to establish that I think there is a trivial transformation to the new system for the basic "well-known service" pattern. The core addon (e.g. Ember Data) can export a well-known token that everyone agrees to use instead of the string. It could literally be Object.freeze({ label: "service:store" }). The app would have to register an override for that service token in an initializer (admittedly a DX regression), then everything else would work. It doesn't work any better and doesn't solve any of the new problems, but it's also no worse. To the extent that the other problems are "acceptable" today, this is probably also acceptable, and if there were plans to address them within the paradigm of today's system, it seems like those should be able to implemented here also.

So building on that, let's see if we can do better and solve the second problem. I think it should be possible. The key points are:

1. Everyone has to use exactly the same (===) service token.
2. Addons (v2 addons at least) only provide code that the app can choose to use, they don't ambiently just "do stuff". So any lazy-loading/module graph concerns are strictly an app concern.

Specifically:

1. It is fine and correct for addons that wants to inject the store with import Store from "@ember-data/store"; as the token.
2. App code should not be importing from that location, because then there is nothing tying the usage of the store in app code to wherever that implementation is located.

Concretely, in addons:

// addon code: @ember-data/graphql

import Store from "@ember-data/store";
import { service } from "@ember/service";

export class GraphqlThingy {
  @service(Store) store;

  doStuffWithTheStore() {
    this.store.stuff();
  }
}

And in app:

// app/services/store.js
// assuming there is no magic happening anymore just by way of placing a file in this location
// it doesn't get into the build unless imported

import Store "@ember-data/store";

Store.implement(
  // implementation of required bits.
  // or subclass from an abstract class, etc
);

// re-export the same well-known store token!
// in practice you would just make `Store.implement` return itself, to make that nicer
// but I want to be explicit about what is ultimately essential and not get bogged down
// by designing the API here
export default Store;

// app/routes/whatever.js

// remember to import from the app location, not from ED directly!
import Store from "my-app/services/store";
import Route from "@ember/route";
import { service } from "@ember/service";

export default class MyRoute {
  @service(Store) store;

  async model() {
    return this.store.find(...);
  }
}

With that, I think the module graph problem is solved. You no longer have to eagerly override the store in an initializer (thereby importing everything into the initial bundle). Or rather, you could say that my-app/services/store is the initializer file. Either way, because the consumption of the store is by definition "kicked-off" by something in the app ("addons don't ambiently do stuff"), as long as the app code is importing from its own location that centralizes all of these dependencies, then it all works out.

An alternative to ED designing it's own API for the Store.implement stuff, we could also provide something more general here (which I have a placeholder issue for in #14):

// app/services/store.js
import Store "@ember-data/store";
import { provide } from "@ember/service";

// Store is just a regular abstract class
export class MyStore extends Store {
  // implement required stuff
  // inherit built-in stuff
  // override built-in stuff
  // etc
}

// This is what I am saying we can provide
provide(Store, MyStore);

// re-export the same well-known store token from ED
// and NOT `MyStore`! Otherwise addons cannot depend
// on ED store correctly
//
// In practice we will probably just make `provide()` return
// the LHS to make that more seamless, but again I want to
// be more explicit here for the time being
export default Store;

The reason this is needed, and isn't doable with the existing APIs, is that override takes the owner as argument, as it is really intended for tests, so that things don't need to be cleaned up between tests/don't leak between tests. However, here in the static module scope you obviously don't have an owner, and this is really a distinct thing that talks about something more global. So it needs a slightly different API.

I'm still thinking through whether this API is truly needed and if it's good for it to exist, but I'm leaning towards yes. But either way, I think ultimately the key point here is that –

1. everyone needs to use the same === token
2. app needs to import from wherever they are defining/providing the implementation to get it in the build
3. if all of the app usages of the service token is indirected through the same place as (2), then everything works

So within those guiding principles we can play around what API design feels the nicest.

Now let's talk about the extensions problem. I think there are two general kind of cases. If the extensions are so core to the store that, in practice, they should always be loaded with the store, then I think you just set everything up in the app's store.js, put those extensions in the store itself (like you do today) and call it good:

// app/services/store.js

import Store "@ember-data/store";

Store.implement(
  // implementation of required bits.
  // or subclass from an abstract class, etc
);

import StoreExtension from "ember-data-addon";

// Not exactly sure what this "does" or need to do
// But just showing that the dependency graph is linked up properly
// Everything that has to do and needs to be part of the store is all loaded in this file
Store.extend(StoreExtension, 
  // any required custom stuff
);

// re-export the same well-known store token!
// in practice you would just make `Store.implement` return itself, to make that nicer
// but I want to be explicit about what is ultimately essential and not get bogged down
// by designing the API here
export default Store;

The second approach is, if things can be more granular, and it is desirable for these additional features to be loaded only in the parts of the app that needs it, then composition is a better choice. By which, I mean the addons can each provide their own independent services that injects the store, rather than being part of it. They would follow the same general pattern as with the main ED store, and perhaps we provide utilities around the pattern.

// app/services/store/graphql.js

import GraphqlStore from "some-addon";
import Store from "../store.js";

GraphqlStore.implement(
  // implementation of required bits.
  // same pattern as the main store
  // design the API in such a way that forces
  // `Store` to be used here such that the
  // dep graph falls out naturally
);

// again if we go with this then you would probably just do
// `export default GraphqlStore.provide(...);`
// what really matters is that the same token is exported here
export default GraphqlStore;

// app/routes/whatever.js

// remember to import from the app location, not from the addon directly!
import GraphqlStore from "my-app/services/store/graphql";
import Route from "@ember/route";
import { service } from "@ember/service";

export default class MyRoute {
  @service(GraphqlStore) graphql;

  async model() {
    return this. graphql.find(...);
  }
}

Here, the graphql stuff is only loaded if and when it is needed. And the app's graphql store definition file imports the app's store definition file (the API should be designed so that has to happen) as its dependency, so it all works out.

In general, I think the compositional approach should be favored, and it naturally takes care of any/most of the TypeScript issues you may have.

That said I think it's possible to do something nice for the case where you centralize things into the Store. App's custom functionalities can be modeled that way too. For example:

// addon code

export interface FooExtension {
  // what you expect from the app
}

// basically what Glint does
declare module "@ember-data/extensions" {
  export default interface Registry {
    "foo": FooExtension,
  }  
}

// app/services/store.ts

// ...

Store.extend("foo", {
  // ...
});

// ...

// usage side, either in addon or app
// this API checks if the extension has been registered/implemented,
// then return the typecasted versions of the store
this.store.withExtension("foo")

It's probably possible to do something with classes too to make it nicer. But ultimately, I think favoring/switching to composition will make a lot of things naturally work better.

I'll see if I can come up with a stripped down version of and prototype type it to see what feels the nicest. But I think this shows that the fundamentals are all there.

[chancancode]

I pushed a proof-of-concept here: https://github.com/chancancode/ember-polaris-service/pull/15/files?diff=unified&w=1

1. I implemented the provide() API
2. ember-storage is an addon that, among other things, provide a well-known service Storage, that apps are supposed to implement
3. ember-storage also has an extension registry system to help provide extensions on the Storage service directly in a type-safe way
4. ember-storage-batch is an addon that creates and consumes such an extension
5. ember-storage-json is an alternative approach, providing a service that composes over the Storage service rather than extending it
6. test-app pulls all of these together, implementing both services with custom extensions to each

I believe this satisfies all of the constraints as I understood them, is fully type-safe and I think the API there is quite nice.