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

Wait for data root before loading filesystem #42

Merged
merged 3 commits into from
Aug 29, 2022
Merged

Conversation

bgins
Copy link
Member

@bgins bgins commented Aug 29, 2022

Description

This PR implements the following features:

  • Adds a checkDataRoot function that checks for a filesystem data root
  • Adds a loading filesystem modal
  • Upgrades webnative to 0.34.1

The checkDataRoot function prevents a newly linked device from attempting to load a filesystem whose data root is not yet available on DNS.

While waiting for the data root and loading the filesystem, the newly linked device displays the loading filesystem modal.

Link to issue

Implements #2

Type of change

  • New feature (non-breaking change that adds functionality)

@bgins bgins requested a review from jessmartin August 29, 2022 16:02
@jeffgca jeffgca self-requested a review August 29, 2022 23:07
@bgins bgins merged commit 53b6884 into main Aug 29, 2022
@bgins bgins deleted the wait-for-data-root branch August 29, 2022 23:09
@jeffgca
Copy link
Contributor

jeffgca commented Aug 29, 2022

Testing with Firefox and Safari / Chrome, it works! Huzzah! I logged #46 to track perf slowness.

Copy link
Contributor

@jessmartin jessmartin left a comment

Choose a reason for hiding this comment

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

Just a few thoughts as I was reading through this. Not urgent! @bgins @therealjeffg

const checkDataRoot = async (username: string): Promise<void> => {
let dataRoot = await webnative.dataRoot.lookup(username)

if (dataRoot) return
Copy link
Contributor

Choose a reason for hiding this comment

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

Guard clause! 💪

if (dataRoot) return

return new Promise((resolve) => {
const maxRetries = 20
Copy link
Contributor

Choose a reason for hiding this comment

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

@bgins Not familiar with TypeScript module best practices, but feels weird to define this constant here. The name is quite clear! I wondering if it should be moved to the top level of webnative.ts and commented? 🤔

Maybe we wait until we need a retry construct somewhere else?

Now that I'm thinking about it, should this retry behavior be handled by the WebNative library, perhaps with an exponential backoff? (future improvement, not for this PR)

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

3 participants