Better types for Adapters/Serializers/adapterFor/serializerFor

[runspired]

Which package(s) does this problem pertain to?

• @types/ember-data
• @types/ember-data__adapter
• @types/ember-data__serializer
• @types/ember-data__store

Relevant Discord Conversation

The Overview

• types for adapterFor/serializerFor don't provide for fallback behaviors, we should fix them to
• schema arguments on adapters and serializers are typed as ModelFactory[K] but should be typed as the schema wrapper
• there is no interface folks can use when implementing their own adapter/serializer, but EmberData has one in the docs.

The Longwinded version

Interfaces for Adapters/Serializers

Let's say you are implementing an adapter in your application and want to implement one of the methods (lets say createRecord)

class MyAdapter {
  createRecord(store, schema, snapshot) {
     // implementation goes here
  }
  static create() { return new this() }
}

If you do this, there isn't an interface to implement. We actually do provide an interface for this in EmberData but it's not importable, but I'd be happy to work towards there being a public types for this sort of thing (since this is what most apps should be doing). @chancancode opened a related issue here #1297 on which I've pointed out that the following paths are probably a fine way for this project to go about this in the near term, as they are where these interfaces are located in the docs:

import { MinimumAdapterInterface } from '@ember-data/adapter';
import { MinimumSerializerInterface } from '@ember-data/serializer';

The Schema problem on existing Adapter/Serializer types

lets say you instead extend one of the existing adapters

class MyAdapter extends RestAdapter {
  createRecord(store, schema, snapshot) {}
}

this will throw a number of TS errors unless you type it exactly like this to match the inherited type signature.

class MyAdapter extends RestAdapter {
   createRecord<K extends keyof ModelRegistry>(
     store: Store,
     schema: ModelRegistry[K],
     snapshot: DS.Snapshot
   ) {}
}

this is problematic for several reasons:

1. similar to other places this has been noted we're forced to use DS.Snapshot, which means we are forced to import all of ember-data. Since that's a private definition anyway we should probably just move that to importing it from the private module that exposes it (we can agree within ember-data to keep it there, and there's precedent within the @ember packages for doing similar for types from the internals that need user use).

The appropriate import would be:

import { Snapshot } from '@ember-data/store/-private';

2. The type ModelRegistry[K] gives you an instance type, but in older versions of EmberData this would have been the actual class definition and not an instance. In newer versions of EmberData (3.28+) this is something else entirely, a schema accessor, which is something we've kinda always tried to hint to people in the docs would happen but which is actually true.

Instead of receiving the class that was defined, you'll receive a wrapper which exposes only schema information. That schema information is obtained from the schema service which may derive it from a Model but definitely does not have to.
that wrapper looks a lot like the static Model class for backwards compat, but it is not one.

Adapter methods typically begin by serializing some data for the request. This causes type errors like this:
Property 'modelName' does not exist on type 'ModelRegistry[K]'

class MyAdapter extends RestAdapter {
   createRecord<K extends keyof ModelRegistry>(
     store: Store,
     schema: ModelRegistry[K],
     snapshot: DS.Snapshot
   ) {
     const serializer = store.serializerFor(schema.modelName); // type-error 
   }
}

For reasons mentioned above, fixing the type to access the static class definition instead of the instance type isn't enough. But assuming that type is fixed, there are still issues with this pattern.

3. The keys of the ModelRegistry are not in-sync with the AdapterRegistry and SerializerRegistry, but need to be.

We (appropriately) type schema.modelName as K extends keyof ModelRegistry. At a glance you might think "what's wrong with that?" The trouble is that we also have an adapter and serializer registry and this method is going to lookup the serializer.

store.adapterFor/serializerFor type limitations

The typings for Store types serializerFor like so:

        serializerFor<K extends keyof SerializerRegistry>(
            modelName: K
        ): SerializerRegistry[K];

This type here is in conflict with how serializerFor looks up serializers, which falls back to the application serializer for any string that does not resolve to a serializer on disc. 99% of the time it will be a user mistake to call this with a string that is both not a modelName and is not a serializer on disc, so having a registry does have value, but it needs to know about this fallback.

This fallback behavior is often used to allow fall-through, it gives the option of a serializer being defined per-type but it's not a requirement.

The workaround I see folks doing is to append to the SerializerRegistry like so:

declare module 'ember-data/types/registries/serializer' {
  export default interface SerializerRegistry {
    application: ApplicationSerializer;
    
    // long list of model names being mapped back to application serializer
    user: ApplicationSerializer;
    post: ApplicationSerializer;
  }
}

But remembering to do this for both the AdapterRegistry and the SerializerRegistry is an easy thing to miss. And even if you do this most of the time, if you forget even once you might encounter this error:

Argument of type 'K' is not assignable to parameter of type 'keyof SerializerRegistry'.
  Type 'keyof ModelRegistry' is not assignable to type 'keyof SerializerRegistry'.
  // .. long list of keys the key did not match

Because Serializer|AdapterRegistry do not inherit the keys of ModelRegistry any model you forget to add to `Serializer|AdapterRegistry will cause this code to break, whether it would ever actually see that modelName or not.

And it's a confusing thing to debug, especially since as the long list of keys for models/serializers grows the prompt becomes less and less legible the more things that it has to "not match".

In Conclusion

1. the types for schema on all adapter/serializer types need to be fixed to give you the wrapper API (which corresponds with the actual class definition of the Model in older ember-data versions), not this instance type you get today nor the type of the class definition (which would be the naive fix). More details on what that wrapper looks like are here

2. the SerializerRegistry (and similarly the AdapterRegistry) should be made to have an automatic fallback for anything that is a key in ModelRegistry, pointing at the ApplicationAdapter. This is doable with the following pattern:

class Serializer {}
class CommentSerializer {}
class PostRecord {}
class CommentRecord {}

interface SerializerRegistry {
    application: Serializer;
    comment: CommentSerializer;
  }

interface ModelRegistry {
  post: PostRecord;
  comment: CommentRecord;
}

type AppSerializer = SerializerRegistry['application'];
type ResolvedSerializerRegistry = Omit<Record<keyof ModelRegistry, AppSerializer>, keyof SerializerRegistry> & SerializerRegistry;

type ResolvedApplicationSerializer = ResolvedSerializerRegistry['application']; // Serializer
type ResolvedCommentSerializer = ResolvedSerializerRegistry['comment']; // CommentSerializer
type ResolvedPostSerializer = ResolvedSerializerRegistry['post']; // Serializer
type UnknownSerializer = ResolvedSerializerRegistry['unk']; // type error because unk is not a correct key

3. We should add some new imports to make sure folks don't need to import from ember-data:

import { PromiseManyArray, PromiseBelongsTo } from '@ember-data/model/-private';
import { Snapshot } from '@ember-data/store/-private';
import { MinimumAdapterInterface } from '@ember-data/adapter';
import { MinimumSerializerInterface } from '@ember-data/serializer';