Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable prop for UI kit components to refetch data #93

Conversation

felipecadavid
Copy link
Contributor

@felipecadavid felipecadavid commented Jul 12, 2023

Description of changes

This PR implements React Query for all the UI Kit components and adds a prop for allowing the user to set a refetch interval

Checklist

Before merging to main:

  • Tests
  • Manually tested in React apps
  • Release notes
  • Approved

Release notes

# Component changes

## Time Series

- Added usage of useTimeSeries hook and ability to provide a refetchInterval prop.

## Leaderboard

- Added usage of useLeaderboard hook and ability to provide a refetchInterval prop.

## Counter

- Added usage of useCounter hook and ability to provide a refetchInterval prop.

# Packages changes

## GraphQL

- Added useCounter, useTimeSeries and useLeaderboard hooks for fetching data.

# App changes

## React sample apps.

- Added a test in every sample app for testing refetchInterval.

## Storybook

- Added Y story for Time Series.

@vercel
Copy link

vercel bot commented Jul 12, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
ui-kit ⬜️ Ignored (Inspect) Visit Preview Jul 13, 2023 7:38pm

@linear
Copy link

linear bot commented Jul 12, 2023

PRO-2152 Configurable prop for UI Kit components to refetch data

We have a customer, Christian, who says

In ui-kit version 0.26.8 the components would re-fetch pretty regularly which was useful to keep the data fresh. In version 0.31.0-rc.1, it appears that re-fetching is not occurring.
Is there a built in way to have the components re-fetch without reloading the page? Ideally setting something like a re-fetch interval.

Can we have a prop for defining a refresh interval? Ideally, we will take into consideration what other libraries do. For example, Apollo Client has a useQuery hook with startPolling and stopPolling:

image.png

react-query has refetchInterval, along with some more options for refetchOnMount, refetchInBackground, etc.:

image.png

react-query's refetchInterval is perhaps the simplest.

Copy link
Contributor

@markandrus markandrus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to use the functionality that GraphQL code generator is already providing us. It will simplify things. Please look at my PR here, and let's build on that: #97

It still needs the React app changes merged in. Can you please help add those to the PR, and help to test/validate?

const [refetchInterval, setRefetchInterval] = React.useState(undefined)

const handleSwitchRefetchInterval = () => {
setRefetchInterval(refetchInterval ? undefined : 1000)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this just be refetchInterval ?? 1000, and in all subsequent places, too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something else I am thinking about: there is a lot of manual work keeping the React examples in sync between 16, 17, and 18. If we diff the files, are they really so different? If not, I think we should consider

  1. Checking in one, canonical version of the CounterConnectedTest.jsx, LeaderboardConnectedText.jsx, etc. (by the way, I just notice that these should be tsx, so we can test the types 🤔).
  2. At build time, copying the canonical version of the CounterConnectedTest.jsx, LeaderboardConnectedText.jsx, etc., into the React 16, 17, and 18 folders (if there are minor patches we need to apply, we can do that).

But this may make maintenance easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think right now there is no difference between the code of each version, I like this idea of having a canonical version of each.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this just be refetchInterval ?? 1000, and in all subsequent places, too?

That wouldn't work, here we just want to switch from undefined => 1000 and from 1000 => undefined
refetchInterval ?? 1000 would set refetchInterval to 1000 when it's undefined but wouldn't set it to undefined when it's 1000

@markandrus
Copy link
Contributor

Closed in favor of #97

@markandrus markandrus closed this Jul 17, 2023
@markandrus markandrus deleted the felipe/pro-2152-configurable-prop-for-ui-kit-components-to-refetch-data branch July 17, 2023 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants