Core Data: Draft a type for an Entity's config paramater

[dmsnell]

See #39025

Please ignore this actual PR because it's only meant as a place to discuss the type and work in progress in #39025.

[adamziel]

The type here is more of

Suggested change

	baseURLParams: { context: Context };
	baseURLParams?: Record<string, string> & { context?: Context }

[adamziel]

We could combine this with WithLiterals to get both :

• Strongly typed entity configuration
• The ability to extract information into the type system

It would read similarly to:

const rootEntitiesConfig : DeepReadonly<EntityConfig[]> = [ /* ... */ ];
type DeclaredConfigTypeUnion = typeof rootEntitiesConfig[keyof typeof rootEntitiesConfig];

[adamziel]

I thought I left a comment with that but I can't find it anywhere – here's the type I used in #39025:

export interface EntityConfig {
	baseURL?: string;
	baseURLParams?: EntityQuery< any >;
	getTitle?: ( record: unknown ) => string;
	key?: string;
	kind: string;
	label?: string;
	name: string;
	plural?: string;
	rawAttributes?: readonly string[];
	title?: string;
	transientEdits?: { blocks: boolean };
}

[adamziel]

This can be null

Suggested change

	getTitle( record: Record ): string;
	getTitle( record: Record )?: string;

[adamziel]

Another word would be a "namespace", since some frameworks refer to database tables as "collections". Namespace isn't perfect too, though – I guess we could use either.

[dmsnell]

yeah it still puzzles me and I think the reason is that this kind value connotes different things at the same time. I'm reluctant to use the term namespace because I think that's a back-justification. one of the effects is namespacing, but it's really not used that way.

• namespace is used as part of the API to get versions of the resources, e.g. wp/v2 or wpcom/v1.1.
• it's clearly being used as categorization or classification when contrasting postType and taxonomy. in cases such as in calling getEntityRecord it definitely feels more like "this general kind of entity record" as a grouping term rather than "the entity records provided by this plugin" which is generally how we use "namespace" elsewhere in the project

probably what's throwing us both off is that getOrLoadEntitiesConfig was built upon kind when in fact that function serves a subtly different purpose; more again an incidental design element related to optimization rather than intent.

[adamziel]

Suggested change

	__unstablePrePersist( record: Record, edits: Edit[] ): Edit[];
	__unstablePrePersist( record: Record, edits: Edit[] )?: Edit[];

[adamziel]

This is optional as well

Suggested change

	transientEdits: {
	transientEdits?: {

[adamziel]

Not sure what this is at the moment, but it's optional

Suggested change

	__unstable_rest_base: string;
	__unstable_rest_base?: string;

[adamziel]

Good question! I don't remember, we'll have to find out.

[adamziel]

This is optional and assumed to be an empty list if missing. Also, I don't think it can be anything else than a string.

Suggested change

	rawAttributes: (keyof Record)[];
	rawAttributes?: string[];

[dmsnell]

that's fine, keyof Record should be a string but this constraint mandates that it's a field we actually define in the Record type. not relevant if we don't parameterize by record, but I've added in here at least for thinking about this type because clearly the config value is in effect parameterized by the record type.

if we remove that inherent coupling we don't sacrifice much, but for example, we miss out on guidance in getTitle.

removing that coupling is a real option. like much of this project these properties tend to be added over time as need arises and so they're not necessarily orchestrated as a unified whole. if practical, I think it would be great to keep it because then you would be able to write getTitle and have it show you the available fields in the record that you could use.

[adamziel]

What else can we call it? This name conflicts with the built-in type Record. Perhaps R wouldn't be too terrible? Or _Record?

[adamziel]

I am not sure if parametrizing this will play along with as const/DeepReadOnly and the lookup table approach. If not, perhaps we could hardcode the key and the context in TypeScript, separately from the config, and then constrain the EntityConfig type to only express these hardcoded values? But that seems backwards, as if the type was the actual config.

[dmsnell]

not sure what I was thinking there with Record. nevertheless, we should consider all of this possibly junk-code because I was laser-focused on the other aspects while writing it out.