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

refactor: request api, renterd and sia central sdks #588

Merged
merged 1 commit into from Apr 17, 2024

Conversation

alexfreska
Copy link
Member

@alexfreska alexfreska commented Apr 16, 2024

  • Sia Central SDK updated to use @siafoundation/request.
  • @siafoundation/renterd-js examples updated to new API.

Copy link

changeset-bot bot commented Apr 16, 2024

🦋 Changeset detected

Latest commit: dafdce2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
explorer Minor
website Minor
@siafoundation/sia-central-js Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member Author

alexfreska commented Apr 16, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @alexfreska and the rest of your teammates on Graphite Graphite

@alexfreska alexfreska force-pushed the refactor_sia_central_js branch 2 times, most recently from 859a281 to d6bd8d8 Compare April 16, 2024 20:21
@SiaFoundation SiaFoundation deleted a comment from graphite-app bot Apr 16, 2024
@n8maninger
Copy link
Member

Is [data, error, response] a common pattern over throwing on an error?

@alexfreska alexfreska force-pushed the refactor_sia_central_js branch 2 times, most recently from 825e581 to 2f48e88 Compare April 16, 2024 22:56
@alexfreska
Copy link
Member Author

alexfreska commented Apr 16, 2024

Is [data, error, response] a common pattern over throwing on an error?

@n8maninger I was shooting to streamline the DX, but indeed these API libraries really should throw errors. The streamlined API is the job of the consumer/user's data fetching library - pretty much all data fetching libraries do this handling and return the error as data (swr, react-query).
I reverted to returning the response and throwing the error so that the user can decide what to do. I extracted the [data, error, response] behaviour into a simple convenience method (to) for use in our apps.

Fun examples

Throw vs return

const [buckets, bucketsError] = await to(bus.buckets())
const [objects, objectsError] = await to(bus.objects())

// vs 

// Ahh, gross scoping and typing
let buckets: Bucket[]|undefined = undefined
let objects: Object[]|undefined = undefined
let bucketsError: Error|undefined = undefined
let objectsError: Error|undefined = undefined
try {
  const { data }  = await bus.buckets()
  buckets = data
} catch (e) {
  bucketsError = e
}
try {
  const { data }  = await bus.objects()
  objects = data
} catch (e) {
  objectsError = e
}

or for parallel fetch:

const [[buckets, bucketsError], [objects, objectsError]] = await Promise.all([to(bus.buckets()), to(bus.objects())])

// vs

const [buckets, objects] = await Promise.allSettled([bus.buckets(), bus.objects()])

// As a bonus, with allSettled there is no type information before the status check, lol
const bucketsData = buckets.status === 'fulfilled' ? buckets.value : undefined
const objectsData = objects.status === 'fulfilled' ? objects.value : undefined
const bucketsError = buckets1.status !== 'fulfilled' ? buckets.reason : undefined
const objectsError = buckets2.status !== 'fulfilled' ? objects.reason : undefined

Object vs Array destructuring

// yay, concise and can choose variable name
const [buckets] = await to(bus.buckets())
const [objects] = await to(bus.objects())

// vs

// not too bad, but a little less convenient
const { data: buckets } = await bus.buckets()
const { data: objects } = await bus.objects()

Go and React both use the tuple syntax to streamline things:

buckets, err := bus.Buckets()
const [buckets,setBuckets] = useState<Bucket[])()

@alexfreska alexfreska merged commit 8e2d399 into main Apr 17, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants