pnp / pnpjs

Fluent JavaScript API for SharePoint and Microsoft Graph REST APIs
https://pnp.github.io/pnpjs/
Other
749 stars 305 forks source link

*Paged* caching does not work correctly with latest 3.23 release, but works just fine in 3.21 #2938

Closed tandrasi closed 6 months ago

tandrasi commented 6 months ago

Major Version

3.x

Minor Version Number

3.23

Target environment

SharePoint Framework

Additional environment details

Expected or Desired Behavior

I'm trying to retrieve a paged list of items from a list. I expect that when I have caching enabled with .using(Caching(...)) that it stores cached data in my local storage and reads it correctly on subsequent calls (i.e. paged results come through).

Observed Behavior

What I see is:

3.21 Works as expected with all caching and even paged results. Thanks to the following bug report, I see paging was fixed as of 3.21 https://github.com/pnp/pnpjs/issues/2836 and it works great! All other paging works as expected. I can see the cached data written to local storage and it is read correctly. Paged results even retrieve their "getNext()" results correctly (thanks again for that fix in 3.21!).

3.22 This version doesn't cache properly at all, but that's not important now. See my previous bug report here #2935 .

3.23 Works like 3.21 with the exception of paged results not coming through correctly. This is as though the "hasNext" or "getNext()" props aren't correctly set or utilised. I have a list of 164 items and only 100 ever get returned when through a cached request. Again, this is working perfectly in 3.21 with all 164 items being returned via paged cache requests. I have tested other libraries with hundreds of items too.

This is the first request prior to cache. As is observed, there are 164 items.

First request prior to cache

These are what subsequent caching request look like. Only 100 items ever get returned.

Subsequent requests after

Steps to Reproduce

As per bug report #2935, I've copied what I had written there.

We are using a huge shared library in house. There are many things that are spread between multiple files and classes. So I'll just copy the imports here and then just copy/paste the main functions.

Please see "Observed Behavior" above as that outlines what I'm experiencing. Basically using the same code with 3.21 works yet it completely fails with 3.23 (regarding caching).

import "@pnp/sp/webs";
import "@pnp/sp/lists";
import "@pnp/sp/items";
import "@pnp/sp/batching";
import "@pnp/sp/sites";
import "@pnp/sp/fields";
import "@pnp/sp/attachments";
import "@pnp/sp/site-users/web";
import "@pnp/sp/profiles";
import "@pnp/sp/search";

Get list items. Uses paging.

public async getListItems(listTitle: string, selectFields: Array<string>, expandFields: string = "", filter: string = "", orderBy: string = "Id", ascending: boolean = false, startingIndex: number = 0, top: number = 0, webUrl: string = "", cachingOptions?: ICachingOptions): Promise<any[]> {
    try {
        const web = this.getWeb(webUrl);
        let itemCollection: PagedItemCollection<any> = null;

        try {
            const query = web.lists.getByTitle(listTitle).items
                .filter(filter).select(selectFields.join(","))
                .skip(startingIndex).top(top).expand(expandFields);

            if (this.shouldCache(cachingOptions?.enable))
                query.using(Caching(cachingOptions.props))

            itemCollection = await query.getPaged();

        } catch (e) {
            // silently continue
        }

        if (EntityHelper.isNullOrUndefined(itemCollection))
            return [];

        if (!itemCollection.hasNext) {
            if (orderBy)
                return EntityHelper.copyAndSort(itemCollection.results, orderBy, !ascending);
            else
                return itemCollection.results;
        } else {
            let results: Array<any> = [].concat(itemCollection.results);

            while (itemCollection.hasNext) {
                itemCollection = await itemCollection.getNext();
                results = results.concat(itemCollection.results);
            }

            if (orderBy)
                return EntityHelper.copyAndSort(results, orderBy, !ascending);
            else
                return results
        }
    }
    catch (e) {
        console.error(e);
        return null;
    }
}
bcameron1231 commented 6 months ago

Hi, yes, unfortunately we've had to revert the functionality for Paging and Caching together. The fix we had put into place to support this caused other issues within PnPjs. As such, we decided to revert the change in Version 3. Please see our recent changelog.

We will be evaluating and implementing a new fix in Version 4 (which we are actively working on).

tandrasi commented 6 months ago

Thanks for getting back to me. I'll stick to 3.21 until further notice. Cheers!

bcameron1231 commented 6 months ago

Closing this issue as answered. If you have additional questions or we did not answer your question, please open a new issue, ref this issue, and provide any additional details available. Thank you!

github-actions[bot] commented 6 months ago

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.