stoplightio / elements

Build beautiful, interactive API Docs with embeddable React or Web Components, powered by OpenAPI and Markdown.
https://stoplight.io/open-source/elements/
Apache License 2.0
1.75k stars 202 forks source link

Add explicit conditional fetching in dev-portal fetching hooks #1564

Open mpodlasin opened 3 years ago

mpodlasin commented 3 years ago

Currently hooks in dev portal allow for conditional fetching, but only by not passing certain parameters, like nodeSlug etc.

This is bad, because:

a) It's implicit and undocumented. Users will literally have to look at docs or source code to realise that's how it works. b) It's not properly expressed in types, where many parameters are required. You can still pass an empty string to a parameter for example, but it's almost a hack/trick.

For context, here is the source code of one of our hooks (note how enabled parameter is controlled):

export function useGetNodeContent({
  nodeSlug,
  projectId,
  branchSlug,
}: {
  nodeSlug: string;
  projectId: string;
  branchSlug?: string;
}) {
  const { platformUrl, platformAuthToken } = React.useContext(PlatformContext);

  return useQuery(
    ['useNodeContent', nodeSlug, projectId, branchSlug, platformUrl, platformAuthToken],
    () => getNodeContent({ nodeSlug, projectId, branchSlug, platformUrl, platformAuthToken }),
    { enabled: nodeSlug && projectId ? true : false }, // HEEEEEEREEEEEE!!!
  );
}

AC

To all of our hooks related to fetching (using react-query) add a second "options" parameter, which has an optional enabled flag, allowing to explicitly enable/disable fetching.

For example a call to a hook should look like this:

useGetNodeContent(parameters, { enabled: false })

In this case the fetch would not happen.

marcelltoth commented 3 years ago

This is missing the use-case. What would we use it for, and is that use-case important enough to justify not implementing it on the consumer side (by using the fetcher function directly) instead?

mpodlasin commented 3 years ago

Yeah, this one can be iceboxes or even closed for now.

For some time I felt I needed it in "redirect from old links" story, but I managed without it.

Although there are places in platform where I believe we are fetching with those hooks even though v7 flag is off.

Would make it easier to not fetch in those cases, instead of creating new components just to make a conditional.

mpodlasin commented 3 years ago

Also note my issue - we already have that functionality, but introduced in the worst possible way - undocumented, untyped, difficult to find out, unless you literally read the source code.