Extend source's Data interface with types from new handlers

Not really, you can use the class as a type for an object with the same props:

class Base {
  isReady = false;
  isFetching = false;
}

const base: Base = {
  isReady: true,
  isFetching: false
}

And you can do an instanceOf of anything, no matter the type:

class Base {
  isReady = false;
  isFetching = false;
}

const noType = {}

if (noType instanceof Base) {
  noType.isFetching = true;
}

Playground for this here.

Good idea, we can do that.

I think it’s not that ugly. And for handlers, it can be great to use new Category({ ... }).

I think we can avoid having any in props and this is not that complex (I guess):

class Base {
  isReady = false;
  isFetching = false;
}

class Taxonomy extends Base {
  isTaxonomy: true = true;
  taxonomy: string;
  id: number;

  constructor(props: { taxonomy: Taxonomy["taxonomy"]; id: Taxonomy["id"]}) {
    super();
    Object.assign(this, props);
  }
}

class Category extends Taxonomy {
  isCategory: true = true;
  taxonomy: "category";

  constructor(props: { id: Taxonomy["id"] }) {
    super({ taxonomy: "category", id: props.id })
  }
}

const tax = new Taxonomy({
  taxonomy: "some-taxonomy",
  id: 123
});

const cat = new Category({
  id: 123
});

This is the playground.

We can also use Pick for the constructor type, which is the opposite of Omit:

class Taxonomy extends Base {
  isTaxonomy: true = true;
  taxonomy: string;
  id: number;

  constructor({ taxonomy, id }: Pick<Taxonomy, "taxonomy" | "id">) {
    super();
    this.taxonomy = taxonomy;
    this.id = id;
  }
}

class Category extends Taxonomy {
  isCategory: true = true;
  taxonomy: "category";

  constructor({ id }: Pick<Category, "id">) {
    super({ taxonomy: "category", id })
  }
}

I think that TypeScript could work in that case, but if you run later

if (base instanceof Base) {
  ...
}

any code inside the if would never be reached, because Typescript’s instanceof is compiled to JavaScript’s instanceof, and that would check if base (which is actually compiled as a plain object) is an instance of Base, i.e. it would look for Base in its prototype chain.

That would fail, right? :thinking:

You are absolutely right :sweat: :persevere: :persevere: :persevere:

I feel stupid :laughing:

We’re back to functional type guards then :+1:

To summarize, people using TypeScript would have to do this:

import { isTaxonomy } from "@frontity/source";

if (isTaxonomy(data)) {
  // ...
}

People extending Data would have to do this:

// packages/some-package/types.ts
import { Package } from "frontity/types";
import Source, { BaseData, Taxonomy, Data } from "@frontity/source/types";

// Just create a class.
export interface NewThing extends BaseData {
  isNewThing: true
}

// Or it can extend other types.
export interface NewThing extends Taxonomy {
  isNewThing: true
}

type ExtendedData = Data | NewThing;

export function isNewThing(data: ExtendedData): data is NewThing {
  return (data as NewThing).isNewThing === true;
}

export interface SomePackage extends Package {
  state: {
    source?: Source<ExtendedData>["state"]["source"]
  }
  //...
}

We can use generics in the Source type to allow for a different Data, like this Source<ExtendedData>;

// packages/some-package/src/components/index.ts
import { isNewThing } from "../../types";
import { isArchive } from "@frontity/source";

const MyTheme = ({ state }) => {
  const data = state.source.get(state.router.link);
  if (isNewThing(data)) {
    // data: NewThing !!
  } else if (isArchive(data)) {
    // data: ArchiveData !!
  }
};

In our side:

state.source.data could be simply an array of BaseData:

interface Source<D = Data> extends Package {
  state: {
    source: {
      data: Record<string, D>;

BaseData would have only the types injected by source on all data objects:

interface BaseData {
  isReady: boolean;
  isFetching: boolean;
  link: string;
  // ...
}

Those would be the only ones accessible without a type guard check:

if (data.isReady) {
  const link = data.link;
  // ...
}

And finally for Source v2 handlers we can use generics too:

import { Handler, Taxonomy } from "@frontity/source/types";
import { ExtendedData, NewThing } from "../../types";

type NewThingBase = Omit<NewThing, keyof Taxonomy>;

const handler: Handler<ExtendedData> = async ({ link }) => {
  const response = await fetch(link).then(r => r.json());
  const { entities } = normalize(response);
  const data: NewThingBase = {
    isNewThing: true;
    // ...
  }
  return { data, entities };
};

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

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

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:

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.

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.

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.

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?

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.

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

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.

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 :blush:

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

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.

Plot twist.

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.

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;