Pnpjs: Issues with taxonomy batch requests - Continued #420

[AJIXuMuK]

Category

• [ ] Enhancement
• [x] Bug
• [ ] Question
• [ ] Documentation gap/issue

Version

Please specify what version of the library you are using: [1.2.8-2]

Please specify what version(s) of SharePoint you are targeting: [Online]

This is a sequel of #420 :)

Expected / Desired Behavior / Question

Batching works with caching

Observed Behavior

Batching is failing if there is caching anywhere in the requests (not even related to current batch).

Steps to Reproduce

Working code

   taxonomy.setup({
      spfxContext: this.context
    });

    const termStore = await taxonomy.getDefaultSiteCollectionTermStore().get();
    const terms = await termStore.getTerms({
      TermLabel: 'Dev',
      StringMatchOption: StringMatchOption.StartsWith,
      DefaultLabelOnly: true,
      TrimUnavailable: true,
      ResultCollectionSize: 10
    }).get();
    console.log(terms);

    let batch = taxonomy.createBatch();

    terms.forEach(term => {
      term.termSet.inBatch(batch).get().then(termSet => {
        console.log(termSet.Id);
      });
      term.labels.inBatch(batch).get().then(labels => {
        console.log(labels);
      });
    });

    batch.execute().then(() => {
      console.log('executed');
    }, (error) => {
      console.log(error);
    });

Now, if I modify the code and change the first request to use caching const termStore = await taxonomy.getDefaultSiteCollectionTermStore().usingCaching().get(); the batch will fail.
Same if I add caching to getTerms or any other request.
The error is Invalid request..

The workaround proposed in #420 works:

let batch = taxonomy.createBatch();
terms.forEach(term => {
      taxonomy.getDefaultKeywordTermStore().getTermById(term.Id).termSet.inBatch(batch).usingCaching().get().then(termSet => {
        console.log(termSet.Id);
      });
      taxonomy.getDefaultKeywordTermStore().getTermById(term.Id).labels.inBatch(batch).usingCaching().get().then(labels => {
        console.log(labels);
      });
    });

The bad thing that in real project I have more complicated scenarios where I'm not always able to construct the whole chain from taxonomy to the objects I want to request.

[patrick-rodgers]

I just published a new beta with this fix included. Please test before tomorrow if you get a chance to ensure this is fixed for the release. I am pretty confident it is, but always nice to get a second opinion in complicated scenarios. Thanks!

[AJIXuMuK]

Thanks @patrick-rodgers!

I will test it a bit later today.

[AJIXuMuK]

It doesn't work :(
Same code as I posted before, new version (1.2.8-3), new error:

Uncaught (in promise) TypeError: Cannot read property 'length' of undefined           util.ts:231
    at getHashCode (util.ts:231)
    at clintsvcqueryable.ts:291

I've also tried fresh project just to be on the safe side of things - same results...

[patrick-rodgers]

I don't get that error when I test the code you pasted above, could you share exactly the code you are running that is failing?

Edit: Let me add, the error is coming because the options.body is apparently undefined - which so far as I know shouldn't happen. But obviously is somehow - so want to understand the code path that results in an empty body in the toRequestContext method.

Edit the second: So I think I recreated it you added usingCaching() to the calls with inBatch() in the terms loop? Which isn't the same code as above - always saves time to get exactly the code you're running. Based on this I think I see where the issue is and why.

[AJIXuMuK]

Sorry, my bad, I changed the code for caching to test workaround (second code sample) and didn't remove the caching afterwards.
Sorry about that.

Here is a link to the sample if you need it...

[patrick-rodgers]

Ok, this is fixed I believe. Publishing a new beta which should be out in a couple minutes. If you get a chance to try that, please do - I think all scenarios are covered now. Ideally this will all be resolved in the next release.

[AJIXuMuK]

Yep, it looks fine from my side as well!

Thank you Patrick!