feat: return Claim from ContentClaimsLocator#locate
#12
Conversation
Add another caching `Map` in `ContentClaimsLocator`. This one caches the fetched content claims and gives callers a way to access the claims that were used as the basis for the `Location` returned by `locate`. I'm not totally convinced this is the right way to handle this, but it does give downstream users the ability to get the raw content claims associated with a CID.
|
I'd return the claims involved as part of return value of |
|
ok great - even easier! updated the PR. Any idea what the test failure might be? it's a very odd one... |
seems to be broken! pnpm/action-setup#135
|
update: figured out the test failure! upgrading the pnpm action fixed it |
travis
changed the title
ContentClaimsLocator#locate
src/locator/content-claims.js
Outdated
| @@ -82,7 +82,8 @@ export class ContentClaimsLocator { | |||
| site: [{ | |||
| location: claim.location.map(l => new URL(l)), | |||
| range: { offset: claim.range.offset, length: claim.range.length } | |||
| }] | |||
| }], | |||
| claim | |||
There was a problem hiding this comment.
This is in a loop - multiple claims can contribute to this "location". I think it needs to be a DigestMap<Claim> or Claim[]
There was a problem hiding this comment.
oh jeez good call - will update!
|
Can you add a test/assertion(s)? 🙏 |
|
I'm going to put a pause on this, pending @Peeja's RFC referenced here: storacha/project-tracking#138 (comment) edit: that said I did work up a test for this! have pushed it but probably worth expanding if we go this route. |
…or#locate we may want to add more assertions here, but I'm putting this on pause for now anyway - can follow up once we have more clarity
|
Pretty sure this is now moot, but we can restore it if we need to. |
Return underlying
ClaimwithLocation- I madeClaimmandatory and it didn't seem to break anything, but I'm not sure if there are cases where we might want to returnLocations without including aClaim- open to making this optional!