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 thisSource<ExtendedData>
. - You won’t use a generic for
Data
in theHandler
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
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;
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
andEntity
and notBase
, 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 thestate.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 ofstate.source.get("/")
though, by allowing people to usestate.source.data["/"]
directly once we have filters.
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
.
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 calledTaxonomyEntity
is going to be renamed toTermEntity
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
andisTaxonomy(data)
are going to be deprecated and aliased todata.isTerm
andisTerm(data)
– it’s funny thatisTaxonomy(data)
is going to be released and deprecated at the same time.
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 haveData
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 haveEntity
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 theTermData
type is nowisTerm
. 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 (...) { ... }