Skip to content
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

.get() return types #199

Closed
2 of 4 tasks
SPWizard01 opened this issue Aug 12, 2018 · 13 comments
Closed
2 of 4 tasks

.get() return types #199

SPWizard01 opened this issue Aug 12, 2018 · 13 comments

Comments

@SPWizard01
Copy link

Category

  • Enhancement
  • Bug
  • Question
  • Documentation gap/issue

Version

Please specify what version of the library you are using: [1.1.4]

Expected / Desired Behavior / Question

Is there a defined set of return data types?
The following code returns Promise<any>:
sp.web.lists.getByTitle("SomeList").get()
and i guess that's fine, however it would be nice to have a set of "defined" return types.
In my project i had to create it myself for example:
let list: IODataListResult: await sp.web.lists.getByTitle("SomeList").get()

declare interface IODataListResult {
"odata.metadata": string;
"odata.type": string;
"odata.id": string;
"odata.etag": string;
"odata.editLink": string;
AllowContentTypes: boolean;
BaseTemplate: number;
BaseType: number;
ContentTypesEnabled: boolean;
CrawlNonDefaultViews: boolean;
Created: string;
CurrentChangeToken: { StringValue: string };
DefaultContentApprovalWorkflowId: string;
Description: string;
Direction: string;
DocumentTemplateUrl: string | null;
DraftVersionVisibility: number;
EnableAttachments: boolean;
EnableFolderCreation: boolean;
EnableMinorVersions: boolean;
EnableModeration: boolean;
EnableVersioning: boolean;
EntityTypeName: string;
FileSavePostProcessingEnabled: boolean;
ForceCheckout: boolean;
HasExternalDataSource: boolean;
Hidden: boolean;
Id: string;
ImageUrl: string;
IrmEnabled: boolean;
IrmExpire: boolean;
IrmReject: boolean;
IsApplicationList: boolean;
IsCatalog: boolean;
IsPrivate: boolean;
ItemCount: number;
LastItemDeletedDate: string;
LastItemModifiedDate: string;
ListItemEntityTypeFullName: string;
MajorVersionLimit: number;
MajorWithMinorVersionsLimit: number;
MultipleDataList: boolean;
NoCrawl: boolean;
ParentWebUrl: string;
ParserDisabled: boolean;
ServerTemplateCanCreateFolders: boolean;
TemplateFeatureId: string;
Title: string
}

Observed Behavior

The following code returns Promise<any>:
sp.web.lists.getByTitle("SomeList").get()

@koltyakov
Copy link
Member

Hey @SPWizard01,

Custom data type can be provided to get method, it has generic types with a default value of any (for the obvious reasons):

image

@SPWizard01
Copy link
Author

Hello,

Yes I am aware of that, however I think it would be nice to have it available within pnp library.
i.e. if i get List metadata the structure of the data would remain the same pretty much always (except with the example you provided, where you use .select() to limit it)

@Katli95
Copy link

Katli95 commented Aug 13, 2018

I've actually been thinking about this lately, what would be the best practice way to convert the response item to a TypeScript object, which might contain attributes with different names from the list columns, thus requiring a mapping of sorts from the SP List data to the TS Model?

example list columns [internal names]:
Title, Dollars, Qnt

example TS Model attributes:
Title, Price, Quantity

@patrick-rodgers
Copy link
Member

Both topics that @SPWizard01 and @Katli95 bring up have been discussed before. I'll try and summarize the previous discussions, and welcome your thoughts.

  1. Why don't we provide interfaces for all the types such as list, web, etc?
    Largely because I don't see a ton of value there, but have always said if someone wants to add them we'll merge the PR. Generally you aren't getting all the properties of a web or list but selecting a few to use. In those cases having a large interface with 50 optional properties doesn't really help much, you aren't benefiting from type checking and you still have to know what properties exist. Better IMO to create your own interfaces custom to your project enforcing the properties you know you care about. There are also differences between SPO, 2013, 2016, and 2019 in those classes. So would we then require multiple interfaces, etc. Seems like a lot of upkeep for minimal actual value. We try to provide interfaces for those cases where it seems helpful.

  2. Why don't you have some type of ORM system built into the library to convert data to entities?
    In short because we would never do it right for all cases, there are just too many ways to solve this problem. What we think is a good ORM system wouldn't work with (let's pretend) Angular well, or React, or [insert some present or future thing]. So we end up chasing our tails trying to create yet another ORM. This would just then shift the question from "why don't you" to "why did you do it this way and not this one." Instead we have provided a generic approach through the spODataEntity and spODataEntityArray parsers. Examples. This allows you to create an object of you own design and handle the mapping any way you want. So far as best practice in this area that would be a matter of opinion. IF you want mine, it is to just use the objects as they come back in JSON, no extra overhead or mapping needed. Some folks have taken a swing at solving this in various ways, here is one example using decorators from @jquintozamora. If there are things we can do to make it easier for folks (such as actually adding a documentation page on this :) ) please let us know.

@patrick-rodgers
Copy link
Member

Do very much want to invite a discussion here, didn't mean to close things down with my comment.

@Katli95
Copy link

Katli95 commented Aug 16, 2018

Could you please elaborate on how the spODataEntity and parser works? I'm relatively new to this and the best method I've worked out is to manually convert the response to an interface of my own design before returning the items from a service class.

@SPWizard01
Copy link
Author

I would assume that Interfaces for such things as list items cannot be defined because there are custom lists/custom content types etc. however those basic(or well known) ones can be done in my opinion.

For me, its no big deal, there are good tools to convert response JSON into interface etc. I just think that in some cases it would help folks to get started.

Maybe you are right maybe its just lack of documentation :D

@patrick-rodgers
Copy link
Member

@Katli95 - you can see the article on the old repo's wiki. I have realized we didn't get it moved over for the new docs so will need to get that corrected.

@SPWizard01 - as mentioned above, will certainly merge a PR if someone wants to build out those interfaces.

@pedro-pedrosa
Copy link
Contributor

In those cases having a large interface with 50 optional properties doesn't really help much, you aren't benefiting from type checking and you still have to know what properties exist.

I would disagree. Something like this would be helpful (building on the OP's interface):

declare function select<K extends keyof IODataListResult>(...fields:K[]): Pick<IODataListResult, K>;

If select is implemented like that, it errors if you try to select a non-existing property, you will get autocomplete on the select arguments, and the resulting type will be filtered down to the properties you selected.

Examples:
select('Ti autocompletes to Title and TemplateFeatureId
select('Title', 'Id'). autocompletes to Title and Id

I fully support this request.

@patrick-rodgers
Copy link
Member

The part of my quote above you didn't include is that I'd certainly look at merging a PR covering this. The only issue I see with this approach is that if fields are added they would effectively be hidden/blocked until the interface is updated. If you want to implement your suggestion, please do. Remember that select is shared on all the collections so we would want to ensure we are creating a solution that will work across all the objects and in a maintainable way.

@pedro-pedrosa
Copy link
Contributor

Now that I think of it, there could be another issue with expands, like select('User/Id').expand('User'). I don't know if these can be defined in TypeScript.

@patrick-rodgers
Copy link
Member

Going to close this as inactive. As mentioned if someone wants to work at creating typings I support it, but I think it is a harder problem than it initially appears. Happy to keep the discussion going if needed, please reopen the thread. Thanks!

@github-actions
Copy link

This issue is locked for inactivity or age. If you have a related issue please open a new issue and reference this one. Closed issues are not tracked.

@github-actions github-actions bot locked and limited conversation to collaborators May 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants