Alpha 11

[danielfdsilva]

Changelog

🎉 Features

• Add pagination support to useCollections

🚀 Improvements

• Allow STAC API request headers to be set via Provider
• Allow useStacApi to return the instance in the context

🐛 Fixes

[danielfdsilva]

Mostly to fix this: content type json error and collections pagination on stac-manager

[oliverroick]

The changes look mostly good, although this is a bit hard to review with that many changes and very little context why we need these changes.

I left a couple of comments and I think we should address the following:

• The style updates are fine, I guess, but shouldn’t the linter be updated to match those preferences?
• Any changes to developer-facing APIs should be documented in the README.

[oliverroick]

If the collections response has next and/or previous links specified, why are we using offset and limit here instead of calling the provided links. Also, the collections spec uses page query params for pagination in collections.

[danielfdsilva]

While working with the eoepca api (https://eoapi.apx.develop.eoepca.org/stac/collections?limit=2) it uses offset and limit, so I went with that.
Using the offset and limit allows the user to go to a specific page, or to reset to the first page more easily. There's no "first" link, so to go back we'd need to get the current link and set the offset to 0.

Not sure what the best approach would be. Any ideas?

[oliverroick]

What I said was not 100% correct. The spec actually says this:

With this, a link with relation next is included in the links array, and this is used to navigate to the next page of Collection objects. The specific query parameter used for paging is implementation specific and not defined by STAC API. For example, an implementation may take a parameter (e.g., page) indicating the numeric page of results, a base64-encoded value indicating the last result returned for the current page (e.g., search_after as in Elasticsearch), or a cursor token representing backend state.

So really, we don't know how individual services implement pagination. Seeing, that STAC React is a generic library, the safest bet is to rely on next and prev/previous links.

Not being able to reliably infer a first or last page link is an obvious flaw, which should probably be added to the STAC API spec. We could try to infer the pagination parameters, but it seems to open a huge can of worms because we really don't know how pagination will work in each instance.

[danielfdsilva]

Hmm, this is unfortunate. I'll check about using the prev/next

[oliverroick]

I don't understand why we need this. useStacApi was an undocumented feature, it wasn't meant to be used by implementers. How will this be used?

[danielfdsilva]

It is basically to access the underlying api instance to make custom requests not covered by the hooks. In this case I'm using it to make the requests needed to update/create collections. Instead of making a call directly with window.fetch I'm getting the api instance to make use of its fetch method which gets the url from context and also has nice error handling.

[oliverroick]

The stacApi object is exposed through the StacApiContext. Would something like this not work?

const { stacApi } = useStacApiContext();
stacApi.fetch("/collections", {
  method: "POST"
  payload: { ... }
  headers: { ... }
})

[danielfdsilva]

Yup, that would work as well. However the useStacApiContext is not exported, and having to change something, it made more sense to me that the useStacApi hook (which is exported), would return the instance in the context.

stac-react/src/index.ts

Lines 8 to 15 in a619465

	export {
	useCollections,
	useCollection,
	useItem,
	useStacSearch,
	useStacApi,
	StacApiProvider,
	};

[oliverroick]

Yes ok, in that case I would actually suggest to not export useStacApi (which is only exported because it was needed in previous versions and I forgot to remove the export) and to export useStacApiContext instead. It makes sense for exactly the developer case you're facing, where you need access to the stacApi instance without the need to change useStacApi and introducing complexity by making the url param optional.

[danielfdsilva]

@oliverroick thank you for the review and apologies for the lack of detail, I'll try to go over the points, but let me know if you want to have a longer discussion about this:

• Linting: the linter automatically formatted the files when I went to fix the errors. Maybe it picked up more than just the project's linter for some reason?
• Readme: Will update.

[danielfdsilva]

@oliverroick Updated to make use of previous/next links if they exist.
Let me know if this looks good and I'll update the documentation.

[oliverroick]

Do we need to expose this? It feels like this should only be used internally.

[oliverroick]

Actually, thinking about it. Do we need the currentUrl state at all? It seems like it's only needed the set the prev/next page URLs, but couldn't we call getCollections directly from the nextPage/prevPage callbacks?

const nextPage = useCallback(() => getCollections(nextUrl), [nextUrl]);
const prevPage = useCallback(() => getCollections(prevUrl), [prevUrl]);

[oliverroick]

@danielfdsilva This looks good, thank you for those changes. Just one minor thing, just wondering if we need to expose setCurrentUrl?