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

Fix race condition with isLoading in use-data.hook.ts #597

Open
lyonsil opened this issue Oct 26, 2023 · 4 comments
Open

Fix race condition with isLoading in use-data.hook.ts #597

lyonsil opened this issue Oct 26, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@lyonsil
Copy link
Member

lyonsil commented Oct 26, 2023

Describe the bug
When changing BCV repeatedly, the resource viewer will occasionally get stuck showing Loading instead of the right part of scripture. It appears to be related to the isLoading flag in this part of the resource-viewer.web-view.tsx:

  return (
    <div>
      {isLoading ? (
        'Loading'
      ) : (
        <ScriptureTextPanelUsxEditor usx={usx ?? '<usx/>'} onChanged={setUsx} />
      )}
    </div>
  );

When the code is changed to ignore isLoading, the resource viewer continues to show scripture properly as expected.

  return (
    <div>
        <ScriptureTextPanelUsxEditor usx={usx ?? '<usx/>'} onChanged={setUsx} />
    </div>
  );

To Reproduce
Steps to reproduce the behavior:

  1. Make sure the resource viewer control is using the isLoading flag from use-data.hook.ts.
  2. Open Platform.Bible with the resource viewer open.
  3. Repeatedly using the < and/or > button to change books and chapters in quick succession.
  4. Eventually the USX viewer will get stuck on Loading. Changing BCV again usually clears it up after one or two updates, but sometimes it takes more.

Expected behavior
The resource viewer should always update to scripture that matches the BCV control.

This is likely an issue with the pattern that people are expected to use isLoading from use-data.hook.ts, not something specifically wrong with the resource viewer per se. Either there is a bug in use-data.hook.ts or the documentation is suggesting the use of return values in a way that is not thread safe across async calls.

@lyonsil lyonsil added the bug Something isn't working label Oct 26, 2023
@lyonsil lyonsil added this to Paranext Oct 26, 2023
@lyonsil lyonsil moved this to Open in Paranext Oct 26, 2023
@tjcouch-sil
Copy link
Member

I guess it has to do with how we're running setIsLoading in the hook in two different async functions. Just feels a bit shifty.

@lyonsil
Copy link
Member Author

lyonsil commented Nov 7, 2023

I saw this in the Text Collection extension today, too. Note the "Loading" text beside HPB despite the fact that HPB was loading fine since you could see the whole chapter to the right of it in the same webview. Unfortunately just changing the BCV doesn't clear it (at least not in this control).

Image

@tjcouch-sil
Copy link
Member

This quite possibly is due to useEventAsync and should be re-evaluated once we fix #1303

@lyonsil
Copy link
Member Author

lyonsil commented Dec 18, 2024

#1303 ended up not needing to touch useEventAsync.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: 📥 For Consideration
Development

No branches or pull requests

2 participants