Skip to content

fix: cached resources implementation#2365

Open
captaincrazybro wants to merge 6 commits into
mainfrom
fix-cached-resources-implementation
Open

fix: cached resources implementation#2365
captaincrazybro wants to merge 6 commits into
mainfrom
fix-cached-resources-implementation

Conversation

@captaincrazybro
Copy link
Copy Markdown
Contributor

@captaincrazybro captaincrazybro commented Jun 4, 2026

Fixed two bugs with the new cached resources implementation

  • Fixed resource text stalled on installation
  • Fixed get resources incorrectly marking resources as needing updates

This change is Reviewable

@imnasnainaec
Copy link
Copy Markdown
Contributor

Devin claims (https://app.devin.ai/review/paranext/paranext-core/pull/2365) this pr introduces "an oscillation between fetchResources = true/false" that potentially "re-triggers the install on every cycle". In the "Prompt for agents", it suggests two different ways to fix it.

(Devin also notes that same issue is already present in extensions/src/platform-scripture-editor/src/model-text-panel.web-view.tsx, so a fix could be applied there too.)

@imnasnainaec
Copy link
Copy Markdown
Contributor

Now supposedly

The old code attempted one install and stopped. The new code creates an unbounded retry loop that consumes network bandwidth and CPU on persistent failures.

Of the two original fix suggestions, I'm personally partial to adding a ref to keep it out of the useEffect dependencies. Something like

useEffect(() => {
  if (
    !fetchResources &&
    isInstalling &&
    dblResourcesProvider &&
    matchDblEntryUid !== undefined &&
    attemptedInstallUidRef.current !== matchDblEntryUid
  ) {
    attemptedInstallUidRef.current = matchDblEntryUid;
    (async () => {
      try {
        await dblResourcesProvider.installDblResource(matchDblEntryUid);
      } catch (e: unknown) {
        logger.error(`Resource panel auto-install failed: ${getErrorMessage(e)}`);
      }
      setFetchResources(true);
    })();
  }
}, [fetchResources, isInstalling, dblResourcesProvider, matchDblEntryUid]);

@captaincrazybro
Copy link
Copy Markdown
Contributor Author

Oh yikes. Sorry I need to think through this more...

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.

2 participants