-
Notifications
You must be signed in to change notification settings - Fork 304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Strongly typed queryable resources #298
Conversation
Thanks @pedro-pedrosa - this is obviously a lot of work! We'll need to review and discuss in detail, but your detailed explanation will be hugely helpful. Will likely be at least few days before I can really dig into this, so please don't take the delay as anything other than me being busy. I have also asked @koltyakov to have a look as well. I am excited to check this out! |
Thanks. The main idea for the PR is to gather feedback and figure out what can/should be done next and whether this is practical or not. If you look at the WebQueryableInterface implementation, it's rather big and hard to digest and I have only declared two expanded properties ( I believe there's still hope as the users of the library will only get a smaller subset of properties in autocomplete when they call For the pnpjs developer sanity, I really think a tool should be developed which would accept JSON objects with the base properties and would output .d.ts files with all the composite properties defined. Something like this: [
{
"name": "WebQueryableInterface",
"type": "SP.Web",
"properties": {
"Title": {
"type": "string",
"expandable": false,
"default": true,
},
"CurrentUser": {
"type": "SP.User",
"expandable": true
}
}
},
{
"name": "UserQueryableInterface",
"type": "SP.User",
"properties": {
"Id": {
"type": "number",
"expandable": false,
"default": true
}
}
}
] Would generate the following .d.ts file: type WebQueryableInterfaceProperties = {
CurrentUser: QueryableProp<any, true>;
"CurrentUser@odata.navigationLinkUrl": QueryableODataProp<string, "CurrentUser">;
"CurrentUser/Id": QueryableCompositeProp<number, "CurrentUser", "Id">;
"CurrentUser/odata.editLink": QueryableCompositeProp<QueryableODataProp<string, any>, "CurrentUser", "odata.editLink">;
"CurrentUser/odata.id": QueryableCompositeProp<QueryableODataProp<string, any>, "CurrentUser", "odata.id">;
"CurrentUser/odata.type": QueryableCompositeProp<QueryableODataProp<"SP.User", any>, "CurrentUser", "odata.type">;
Title: QueryableProp<string, false>;
"odata.editLink": QueryableODataProp<string, any>;
"odata.id": QueryableODataProp<string, any>;
"odata.metadata": QueryableODataProp<string, any>;
"odata.type": QueryableODataProp<"SP.Web", any>;
};
type WebQueryableInterfaceDefaultSelected = "CurrentUser/Id" | "Title";
type WebQueryableInterfaceDefaultExpanded = never;
type WebQueryableInterface = QueryableInterface<WebQueryableInterfaceProperties, WebQueryableInterfaceDefaultSelected, WebQueryableInterfaceDefaultExpanded>;
type UserQueryableInterfaceProperties = {
Id: QueryableProp<number, false>;
"odata.editLink": QueryableODataProp<string, any>;
"odata.id": QueryableODataProp<string, any>;
"odata.metadata": QueryableODataProp<string, any>;
"odata.type": QueryableODataProp<"SP.User", any>;
};
type UserQueryableInterfaceDefaultSelected = "Id";
type UserQueryableInterfaceDefaultExpanded = never;
type UserQueryableInterface = QueryableInterface<UserQueryableInterfaceProperties, UserQueryableInterfaceDefaultSelected, UserQueryableInterfaceDefaultExpanded>; This would allow a developer to write the |
@pedro-pedrosa, @patrick-rodgers, will also take a look. Something which requires a couple of coffee cups to wrap around the head and a night to sleep with. My main concern about strongly typed responses is related to the fact that the response for the same object can be different due to different OData modes and across different versions of SharePoint and have many nuances. Strongly typed responses, in my opinion, is something which would be great if generated against provided user schema or based on AOT requests to the particular environment with some manual reducing afterward, and stored in the context of user project declarations. Yet, some universal/trimmed declarations can be super handy to have inside the library. We discussed something similar with @s-KaiNet some time ago, yet never go deeper with making it work. P.S. @pedro-pedrosa, really lot of work, you did a huge step forward. |
I do understand the concern regarding different SharePoint environments. I don't know if it's possible to provide the typings in separate packages inside Alternatively the user would be able to use The generic approach of having the broadest possible set of properties for all types is not ideal, I think for the SPO developer it would be great but for the 2013 developer it would be painful to discover something is missing from their REST API only when they run the code. As for automatic generation, I know there's the
Then I merge everything into a single set and do another REST call selecing all of those properties. The ones that return something are the ones that work. Then I parse the response and write the type definition. This process can be automated maybe. To generate these typings one would need an instance of each of SPO, 2010, 2013, 2016, 2019 and possibly others (like service packs or commulative updates). But it's definitely doable and sounds like the way to go in terms of maintenance of code. |
Category
Related Issues
Builds on suggestions made on #199
What's in this Pull Request?
This PR aims at providing a strongly typed interface around queryable methods, specifically
select
,expand
,orderBy
, andget
. The main goals of the this PR are:How does it work?
Currently, queryable resources can specify the result type of their
get
method. This is typically set toany
for single value resources inheriting fromSharePointQueryable
andany[]
for collection resources inheriting fromSharePointQueryableCollection
. Theoretically, it is possible to create an interface with all properties needed for the current use case, but those interfaces are static and not re-usable.This PR introduces the
QueryableInterface
interface which allows you to specify the set of all possible properties of a resource object, including expandable properties. In this PR, as an example, I provide a partial implementation of theWebQueryableInterface
for theWeb
andWebs
classes.The
QueryableInterface
is a triple of:With the above information regarding the queried resource, it is possible to define the type that describes the result of the REST API call. This is achieved by using mapped types and conditional types.
All of these types were made with backwards-compatibility in mind. If the queryable class has been created with a type that is not a
QueryableInterface
, such asany
, the default behavior is provided without loss of functionality.Creating a
QueryableInterface
typeType Properties
To create a
QueryableInterface
type for a resource, one must declare the complete set of properties that can be retrieved from that resource, including expandable properties.Example (this only implements the SP.Web type partially):
Regular properties are typed as
QueryableProp<PropertyType, Expandable>
. Properties accessible by first expanding a parent object are typed asQueryableCompositeProp<PropertyType, ParentPropertyName, ChildPropertyName>
. OData properties are typed asQueryableODataProp<PropertyType, TriggerParentProp>
.Default set of selected properties
The second type of the
QueryableInterface
triple is the set of selected properties. To makeget
work without selecting any properties, the default set of selected properties must be defined as a union of string typesExample (partial example for SP.Web):
The default properties of expandable properties must also be specified so they will apear if
expand
is called without any selects.This set is replaced when
select
is called with a new set of selected properties.Default set of expanded properties
This set is empty for most resources, so the typical implementation is
never
:This set is replaced when
expand
is called with a new set of expanded properties.Creating the triple
After declaring the three types for the
QueryableInterface
, the final type can be declared as follows:Using the
QueryableInterface
typeChanges to
select
The new signature of the
select
methodof theSharePointQueryable
type is as follows:QueryableSelectableKeys
is a mapped type that will return the set of properties that can be selected from the currentQueryableType
. It is similar tokeyof QueryableType
but hides composite properties of non-expanded parent properties (i.e. you can only select"Parent/Id"
if you expand"Parent"
first) and also hides odata properties.Instead of returning the
this
type, it now returns aSharePointQueryable
configured to theQueryableSelect
of the base type with the new set of selected properties. In a nutshell,QueryableSelect
just replaces the set of selected properties from theQueryableInterface
triple.Changes to
expand
Similarly,
expand
was changed to the following:QueryableExpandableKeys
returns the set of expandable properties andQueryableExpand
changes the set of expanded properties of theQueryableInterface
triple.Changes to
get
The signature of
get
was changed to:QueryableGet
accepts aQueryableInterface
type and returns the type that is returned by the REST API call.Example (using above definitions):
webObject
will have the following type:Backwards compatibility
All of these types first check if the given type is
QueryableInterface
. If it isn't, the returned types should adhere to the expected ones (any
).Because
select
will only accept expandable composite properties if the expandable property has been expanded,expand
has to be called beforeselect
. This would be a breaking change. To overcome this, users of the library have to opt-in by callingweb.stronglyTyped()
first.Known issues
Incomplete typings
As previously mentioned, the provided
WebQueryableInterface
is not complete and serves only as an example of the capabilities of this type system.The idea is to review this PR and get feedback in order to develop a more complete set of interfaces after the main structure of the type system is accepted.
Expand references
Linked to the above, it would be ideal to re-use the property types when there are cross-type references. For instance, it would be nice to re-use the properties for the
User
object on any interface that contains properties of theUser
type.This is difficult to do with the current type system because sub-properties must be declared as composed properties on the parent object (i.e. properties like
"CurrentUser/Id", "CurrentUser/Title"
and so on) making us have to explicitly define them every time they are used.Currently, there is no way to create a mapped type with composite keys in typescript (see microsoft/TypeScript#12754). Maybe this can be overcome with an automated tool that takes these types and their relationships and automatically creates .d.ts files with these declarations.
Nested expands
It is possible in REST to use nested expands such as
sp.web.expand("Webs/Webs/Webs").select("Webs/Webs/Webs/Title").get()
. This makes typing such types a lot harder.The suggested approach would be to only type up to 1 or 2 levels deep.
TypeScript version
This solution makes use of mapped types, which were only introduced in TypeScript version 2.8.0. I didn't test the lowest version compatible with these types but I assume it would be 2.9 or 3.0 because of some bug fixes made to TypeScript.
This creates a problem for SPFx solutions which use version 2.4.2 by default (generated using the 1.6 toolchain)
To use these new typings exported by the library, users would have to set up the default TypeScript version used by the toolchain. This can be done by installing the latest TypeScript version locally as a dev dependency using
npm i -D typescript@latest
and editing the gulpfile as follows:Nontheless, this is a breaking change.
select
andexpand
return typesBecause
SharePointQueryable.select
doesn't returnthis
anymore, afterselect
is called, it is not possible to call specific methods of sub-classes.For instance, it is not possible to do the following:
This is currently allowed by the library even though it doesn't make sense, so I believe not supporting this use case is acceptable, still a breaking change.
Also because of this,
select
andexpand
have been overriden inSharePointQueryableCollection
:This makes
SharePointQueryableCollection
not assignable toSharePointQueryable
, which forced a refactoring of the base class of sharepoint queryables, which is why I created theSharePointQueryableBase
class.Other considerations
This PR is simply a request for feedback from members of the pnpjs project and the community as well. Maybe this can be pulled into a separate branch while being developed and then launched as a major version in the future since it contains breaking changes.
It is also possible to pull this into the global dev branch and accept the main breaking change which is the use of the latest TypeScript version (if no other breaking changes are found). The
Web
andWebs
queryables are still typed asany
by default and, currenty, to use this type system, users of the library have to opt-in by callingweb.stronglyTyped()
first. This would be deprecated in a future major version and would be used as the default.