Extend source's Data interface with types from new handlers

luisherranz · 25 February 2020 15:49

This is my last playground in case you want to check something.

orballo · 26 February 2020 06:30

Why would you pass Data as a parameter to Handler? Shouldn’t you pass a Package?

luisherranz · 26 February 2020 07:59

Yeah, well… Source needs Data for both state.source.get and state.source.data so maybe it’s easier for people extending Data to have a generic than to redefine those things themselves:

github.com

frontity/frontity/blob/master/packages/source/types.ts#L33-L34


get: Derived<T extends null ? Source : T, (link: string) => Data>;
data: Record<string, Data>;

How are you doing that right now?

Modifying Package is only needed for source packages, like wp-source so it’s going to be less common.

So maybe we could do this: Source<MyData, MyPackage>. Both optional of course.

orballo · 26 February 2020 08:15

I see. Yeah, I guess that the most common need is to extend data so it makes sense to create a shortcut for that. But in the case of the handler, when you need to pass the Package as well, you’ll have Data defined in both generics. Not sure if that makes sense. I’d stick to only passing Package as generic, so you only worry to wire your custom data to one place, and after that, anything else will just use your package.

I mean, this way you’ll be reducing all the scenarios to just one, which seems more simple to me. If you implemented Handler with two generics, people will need to understand that there is one scenario where you are only extending Data in Source, and that’s the only custom thing you’ll use in the Handler, and the second scenario where you might have extended Post too, and you need both in the handler so you need to pass ExtendedData and CustomPackage.

orballo · 26 February 2020 11:34

This is what I’d recommend to do in every types.ts file of a package as a good practice:

// Types

import WpSource, { Handler as WpSourceHandler } from "@frontity/wp-source/types";

export default interface Theme extends Package {
  name: "my-theme";
  state: {
    theme: {};
    source?: WpSource<ThemeData>["state"]["source"];
  };
}

export type FC<Props extends object = {}> = React.FC<
  Connect<Theme, Props>
>;

export type Handler = WpSourceHandler<Theme>;

// Component

import * as Theme from '../types';

const Component: Theme.FC = () => <div></div>;

// or

import { FC } from '../types';

const Component: FC = () => <div></div>;

// Handler

import * as Theme from '../types';

const handler: Theme.Handler = {
  name: "my-handle",
  pattern: "/custom-path",
  func: async () => {}
};

// or

import { Handler } from '../types';

const handler: Handler = { ... };

And that’s all they need to use while developing in my experience.

EDIT:
I also think that recommending to create aliases of types in the types.ts file is a good idea, because it can become a bit difficult to keep in mind all the packages that are used and need to be imported, and this way they only need to think about the typing once when creating the basic types.ts file and then just importing from there whatever they need.

Also this basic aliases can be generated automatically with the frontity CLI so developers can start working from there.

luisherranz · 26 February 2020 19:15

Just to make sure I understand you correctly. You think that:

Using a generic for Source is fine, like this Source<ExtendedData>.
You won’t use a generic for Data in the Handler type, you’d pass only the package, like this: Handler<MyPackage>.

Is that true?

By the way, are you using the types of WpSource instead of Source to populate state.source? If so, is there any reason?

orballo · 26 February 2020 19:35

That’s correct.

Well, because the API of WpSource extends Source and includes Handler. I assumed that I should use the types of the package I have installed, and not those of the base.

EDIT:
I didn’t update yet, so I don’t know if this has changed.

luisherranz · 27 February 2020 10:22

No it hasn’t, but that’s useful information for Source v2. Thanks!

luisherranz · 15 September 2020 20:03

It seems like we may need to use the same approach for Entities as well as for Data in the case of state.source.entity(link).

More information in the FD that introduced this API: Support for Yoast plugin REST API fields.

David · 30 September 2020 10:57

Hey, I have an idea that would not require devs to extend Data and it seems to work nicely.

Actually, extending Data has a big problem. Imagine that we extend Data to MyData:

type Data = Taxonomy | Post; // The ones you are used to.

type MyData = Data | NewThing; // New data with property `isNewThing`.

export interface SomePackage extends Package {
  state: {
    source?: {
      get: Derived<SomePackage, (link: string) => MyData>;
    }
  }
  //...
}

If you wanted to use the default guard functions that we will have already defined in @frontity/source they wouldn’t work, because the would expect an argument of type Data instead of MyData.

// This is how `isTaxonomy` is supposed to be defined.
// Note that it is using an argument of type `Data`, not `MyData`.
function isTaxonomy(data: Data): data is Taxonomy {
  return (data as Taxonomy).isTaxonomy === true;
}

/**
 * The next check would return this TypeScript error:
 *
 * Argument of type 'MyData' is not assignable to parameter of type 'Data'.
 *  Type 'NewThing' is not assignable to type 'Data'.
 *    Type 'NewThing' is missing the following properties from type 'Post':
 *    isPostType, isPost, type, id
 */
const data = state.source.get("/some/link");
if (isTaxonomy(data) { ... } // TypeScript error!

Okay, but what about not using Data anymore?

Let’s say we define these data types: Taxonomy and Post, both extending Base.

interface Base {
  isFetching: boolean;
  isReady: boolean;
}

interface Taxonomy extends Base {
  isTaxonomy: true;
  taxonomy: string;
}

interface Post extends Base {
  isPostType: true;
  isPost: true;
  type: "post";
  id: number;
}

Instead of using Data in the guard functions, we could use Base.

function isTaxonomy(data: Base): data is Taxonomy {
  return (data as Taxonomy).isTaxonomy === true;
}

function isPost(data: Base): data is Post {
  return (data as Post).isPost === true;
}

And, for state.source.get(), we could set the return type as Base. That type has the common properties of every data object and you could check isReady or isFetching. If you want to use another property you will need to use a type guard function to cast the data object in the same way as if it had returned an object of type Data, so we are not gaining anything using Data here.

Same with state.source.data. As long as any object you are adding there extends from Base, you could add whatever you want.

export interface Source {
  state: {
    source: {
      // Return an object of type `Base`.
      get: Derived<Source, (link: string) => Base>;
      // You can store here any object that extends `Base`.
      data: Record<string, Base>;
    }
  }
  //...
}

Then, if you want to add your own data types, it would be as simple as creating your own interface and its guard function.

// `NewThing` should extend `Base` (or any other type extending `Base`).
interface NewThing extends Base {
  isNewThing: true;
}

// The argument is of type `Base`.
function isNewThing(data: Base): data is NewThing {
  return (data as NewThing).isNewThing === true;
}

That’s the only thing you would have to do, and this would work out of the box:

// `data` here is of type `Base`.
const data = state.source.get(link);

// You can check things as `isReady` or `isFetching`.
if (!data.isReady) return null;

// The following type guards will work as expected.
if (isTaxonomy(data)) {
  data.isTaxonomy;
  data.taxonomy;
} else if (isPost(data)) {
  data.id;
  data.isPost;
  data.isPostType;
} else if (isNewThing(data)) {
  data.isNewThing;
}

@dev-team, I would like to hear your feedback

EDIT: I can’t think of any drawback for this. Let me know if you see any problem.

David · 30 September 2020 11:01

Of course, we could use the same solution for entities here.

Regarding what @luisherranz mentioned in Support for Yoast plugin REST API fields, I would prefer same names and different entry points for entities and data objects than different names.

orballo · 30 September 2020 11:42

Plot twist.

David · 30 September 2020 12:56

Uhm, maybe the type Base I mentioned before could be renamed to Data in case some packages depend on it.

// This would be the `Base` data object.
import { Data } from "@frontity/source"`

I did a quick search and it looks like none of our packages use it, though.

David · 30 September 2020 13:37

The only drawback I found with this approach is that you lose type safety when you add a new data object into state.source.data directly unless you specify it explicitly.

Given the following interfaces and data object map:

interface Base {
  isFetching: boolean;
  isReady: boolean;
}

interface PostType extends Base {
  isPostType: true;
}

const data: Record<string, Base> = {};

This won’t work:

data["/some/link"] = {
  isFetching: true,
  isReady: false,
  // TypeScript Error: Object literal may only specify known properties
  isPostType: true
};

This works as expected and you have type safety:

// Explicitly specify the type.
const post: PostType = {
  isFetching: true,
  isReady: false,
  isPostType: true
};

// `PostType` extends from `Base` so this works.
data["/some/link"] = post;

This surprisingly works as well, but you lose type safety.

// Not needed to specify a type here.
const post = {
  isFetching: true,
  isReady: false,
  isPostType: true
};

// TypeScript doesn't complain.
data["/some/link"] = post;

luisherranz · 1 October 2020 07:33

That’s perfect David! Actually, I feel a bit embarrassed as this is basic OOP. I don’t know how it took us so long to figure this out! So thanks for showing the way

Regarding Data and Entity types:

I agree to name the basic types Data and Entity and not Base, to differentiate between both.
I agree that two separate entry points can be the best approach. The only drawback is that they will conflict with each other when used together, but I guess that’s a minor issue:
```
import { isPostType } from "@frontity/source/data";
import { isPostType } from "@frontity/source/entity";
```
Also, if we create the data entry point, we will have to stick to the state.source.data naming.

I started a conversation to rename data in this thread Rename `source.get` and `source.data` to `source.info` but no proposal was actually better than data, so I’m ok sticking to it. I’ll try to get rid of state.source.get("/") though, by allowing people to use state.source.data["/"] directly once we have filters.

David · 20 October 2020 20:20

TL;DR

We decided to deprecate state.source.entity() as it is not a reliable way to get typed entities. Instead, the recommended way would be to get a typed data object – using data type guards – and access state.source for the specifically related entity.

Type guards for entities are going to be implemented as well – just in case they are useful – but their usage will not be promoted.

It isn’t a widely-used function (it’s not even documented yet) so, it is not such a loss after all.

Typed entities

Currently, there are two ways to get entities from the state:

1. Directly from `state.source[entityType]`

This is just getting entities to the map of entities when you know entity type and entity ID:

const post = state.source.post[60];
const category = state.source.category[7];
const myCPT = state.source.myCPT[123];

In the cases specified above you already have typed entities, because types are enforced for those properties in the @frontity/source package (note that these types can easily be extended by other packages).

But what about getting the entity using the information stored in a link data object?

const unknown = state.source[data.type][data.id]; // ??

There is no way to know the type of that entity unless you constrain the possible values of data.type.

if (data.type === "post" || data.type === "page") {
  const postOrPage = state.source[data.type][data.id];
}

// This would be similar to the previous code
// as the `data.type` prop would be typed as well.
import { isPost, isPage } from "@frontity/source/data";

if (isPost(data) || isPage(data)) {
  const postOrPage = state.source[data.type][data.id];
}

What happens if you only know the general type of that data object?

import { isPostType } from "@frontity/source/data";

if (isPostType(data)) {
  const unknown = state.source[data.type][data.id]; // ??
}

Then, you can’t get a typed entity. Of course, you can cast that entity to PostTypeEntity because you know it is a post type entity, but it’s not the ideal API.

import { isPostType } from "@frontity/source/data";
import { PostTypeEntity } from "@frontity/source/types/entities";

if (isPostType(data)) {
  // Not great, not terrible.
  const unknown: PostTypeEntity = state.source[data.type][data.id];
}

This case will be solved in a future version of @frontity/source. The idea is to make entities of the same type to be under their general type in the state.

interface Source {
  state: {
    source: {
      postType: {
        post: Record<number, PostEntity>;
        page: Record<number, PageEntity>;
        [s: string]: Record<number, PostTypeEntity>;
      };
      // ... api, etc.
    };
  };
}

const source: Source = {
  state: {
    source: {
      postType: {
        post: { ... },
        page: { ... },
      },
      //... api, etc.
    },
  },
};

That change could be backward-compatible making derived views for post, page, etc. maps so they could be accessed as they currently are.

Another option would be to use type guards as we did for data objects.

2. Using `state.source.entity()`

The API of state.source.entity() is similar to state.source.get(), it receives a link and return an entity in this case.

const entity = state.source.entity("/some/link/");

That entity is the main one represented by that link, i.e. if the link points to a post is that post, if it points to a page of some category is that category, etc. There are, however, some links that do not point to a specific entity, like a date archive ("/2020/03/15") or some custom links ("@comments/60").

In this case, it is not easy to know the type of the returned entity.

const unknown = state.source.entity("/some/link/"); // ??

We currently have two approximations on this:

2.1. Entity type guards

Use type guards for entities, the same way we did for data objects returned by state.source.get().

This would be to create a group of type-guard functions that would receive an entity as an argument and would return true or false if that’s the type of the entity.

The following code is a hypothetical component that renders which entity is currently being read.

import React from "react";
import { connect, decode } from "frontity";

// These are `entity` type guards, not `data` type guards.
import { isPostType, isTaxonomy } from "@frontity/source/entities";

const YouAreReading = connect(({ state }) => {
  // Use the current link to get the entity.
  const entity = state.source.entity(state.router.link);

  if (!entity) return null;

  // Get different props depending on the type.
  let name: string;
  let description: string;

  if (isPostType(entity)) {
    name = decode(entity.title.rendered);
    description = decode(entity.excerpt.rendered);
  } else if (isTaxonomy(entity)) {
    name = entity.name;
    description = entity.description;
  } else { 
    // Handle other cases...
  }

  // Render the react component.
  return (
    <Card>
      <Name>{name}</Name>
      <Description>{description}</Description>
    </Card>
  )
});

The thing is that we are not gaining much here using state.source.entity() as we would have to ask in order to distinguish between different entity types, so you would end up having a similar code if you would have used state.source.get() instead.

Also, those type guards wouldn’t be straightforward to implement, as there is neither a common property with a different value for each of them nor a specific property for each entity type that would allow us to easily get the type of an entity.

2.2. Pass `data` to `state.source.entity()`

The main idea here is: as we already have a way to get the type of a specific data object using our own type guards (e.g. isPostType(data)), maybe it could be possible to pass a typed data object to state.source.entity() so it returns a typed entity object.

For example:

// Get the data object associated to a link.
const data = state.source.get(link);

if (isPost(data)) {
  // As we know the type of `data` is `PostData`, it could be
  // inferred that the type of `post` is `Post`.
  const post = state.source.entity(data);
}

I thought that maybe using an overloaded function interface would do the trick:

interface DataToEntity {
  (data: PostData): PostEntity;
  (data: PageData): PageEntity;
  (data: PostTypeData): PostTypeEntity;
  // ... etc.
}

The state.source.entity() derived prop would have to be defined this way:

interface Source {
  state: {
    source: {
      entity: Derived<Source, DataToEntity>
    }
  }
}

This wouldn’t be easily extended – you would have to redefine state.source.entity each time a new Data and Entity types are added.

Also, in this case, it wouldn’t work as expected because overloaded functions don’t allow a type union as an argument unless you specify it (there is an open issue for a long time in TypeScript about that.). For example, you may expect state.source.entity(data) to return an entity of type PostEntity | PageEntity in the following case, but it doesn’t:

const data = state.source.get(link);

if (isPage(data) || isPost(data)) {
  const unknown = state.source.entity(data); // ??
}

I already discussed this with @luisherranz and we took the decision of deprecating state.source.entity() as it turned out not to be a good API, at least for TypeScript developers using the @frontity/source package.

Next steps

Deprecate the API and update packages using it.
Finish writing the entity type guards.
Decide how to expose type guards from @frontity/source.

David · 26 October 2020 17:11

During the meeting where we discussed how to expose type guards from @frontity/source, we agreed on doing the following changes:

All type-guards are going to be imported from the root of @frontity/source.
```
import { isPostType, isArchive } from "@frontity/source";
```
Names for entity type guards will end with Entity and will match the entity type name.
```
import { isPostEntity, isPageEntity } from "@frontity/source";
```

The name TaxonomyEntity is going to be used for those entities that come from /wp/v2/taxonomies. What we called TaxonomyEntity is going to be renamed to TermEntity in order to match WordPress naming. TermEntity will be the type of categories, tags, etc.

/**
 * Entities from /wp/v2/taxonomies.
 */
export interface TaxonomyEntity extends Entity {
  description?: string;
  hierarchical?: boolean;
  name?: string;
  slug?: string;
  rest_base: string;
  types: string[];
}

/**
 * Entities from /wp/v2/categories, /wp/v2/tags, etc.
 */
export interface TermEntity extends Entity {
  id: number;
  count?: number;
  description?: string;
  link: string;
  name?: string;
  slug?: string;
  taxonomy: string;
  parent?: number;
  meta?: Record<string, unknown>;
}

data.isTaxonomy and isTaxonomy(data) are going to be deprecated and aliased to data.isTerm and isTerm(data) – it’s funny that isTaxonomy(data) is going to be released and deprecated at the same time.

David · 29 October 2020 17:08

Final implementation

I’m going to do a recap with the changes that were made in Update to TypeScript 4.0 #449.

TypeScript interfaces

All types are exported from @frontity/source/types.
Every data interface inherits from Data with all the common properties, and have Data appended to its name.
```
import { PostData, PageData } from "@frontity/source/types";
```
Every entity interface inherits from Entity, which is basically a map of unknown properties, and have Entity appended to its name. Also, they were refactored to match the schemas that WordPress exposes in the REST API.
```
import { PostEntity, PageEntity } from "@frontity/source/types";
```
Some entities were renamed:

Before Now

TaxonomyData TermData

TaxonomyEntity TermEntity

PostType TypeEntity

TaxonomyType TaxonomyEntity
The isTaxonomy conditional tag in the TermData type is now isTerm. The previous name is maintained also for backward compatibility so both properties are true in objects of that type.

Type guards

The proper way of typing data objects in Frontity now is using type guards, which are functions that type the given argument. There are several of them for data and entity types, and are accessible from the root of the package:

import { isPost, isPage } from "@frontity/source";

// Inside a connected component...
const data = state.source.get(link);
if (isPost(data)) {
  data.isPost; // true, data is typed.
} else if (isPage(data)) {
  data.isPage; // true, data is typed.
}

Custom types

You can create your own data or entity types simply by extending the base interface (or any other type that inherits from it). For example, to create a custom Data type, you would have to define:

An interface extending Data:

import { Data } from "@frontity/source/types";

interface CustomData extends Data {
  isCustomData: true;
  customProp: string;
}

A type guard for that interface:

const isCustomData(data: Data): data is CustomData {
  return (data as CustomData).isCustomData;
};

Then, you can use it in the code:

import { isPost, ... } from "@frontity/source";
import { isCustomData } from "./custom-data";

// Inside a connected component...
const data = state.source.get(link);
if (isPost(data)) {
  data.isPost; // true, data is typed.
} else if (isCustomData(data)) {
  data.isCustomData; // true, data is typed.
} else if (...) { ... }

luisherranz · 31 October 2020 10:11

Awesome work @david!!

SantosGuillamot · 16 November 2020 10:02

This feature has been included in the latest release : Read the full Release Announcement .

Before	Now
`TaxonomyData`	`TermData`
`TaxonomyEntity`	`TermEntity`
`PostType`	`TypeEntity`
`TaxonomyType`	`TaxonomyEntity`