Skip to content

avoid reference errors when dynamically loading hooks after create ho…#933

Merged
infiton merged 1 commit intomainfrom
fix-self-referential-assignment
Dec 3, 2025
Merged

avoid reference errors when dynamically loading hooks after create ho…#933
infiton merged 1 commit intomainfrom
fix-self-referential-assignment

Conversation

@infiton
Copy link
Contributor

@infiton infiton commented Dec 3, 2025

Users have run into issues of production builds of their application when after initial react router page load they navigate to a route which imports a new hook. A fix for this was added in #926 however it seems in many cases this leads to a ReferenceError: Cannot access 'useFindFirst' before initialization

This is due to something I just learned today had a name temporary dead zone

The issue is that when we i.e. import { useFindFirst } from "@gadgetinc/react" when the react provider has been set up BUT none of the hook modules have been imported we end effectively running this code:

let useFindFirst = createHookStub("useFindFirst", () => {
  useFindFirst = () => { ... }
})

this immediately invokes the inner function in the block that is defining useFindFirst and this tries to assign to useFindFirst which is in the TDZ

the fix is to keep the exported useFindFirst binding stable and have createHookStub update an internal impl variable, rather than doing useFindFirst = … inside the initializer.

PR Checklist

  • Important or complicated code is tested
  • Any user facing changes are documented in the Gadget-side changelog
  • Any immediate changes are slated for release in Gadget via a generated package dependency bump
  • Versions within this monorepo are matching and there's a valid upgrade path

@infiton infiton requested a review from airhorns December 3, 2025 19:55
@infiton
Copy link
Contributor Author

infiton commented Dec 3, 2025

verified on my reproduction app that using the patch versions from #934 fixed the issue

Copy link
Contributor

@airhorns airhorns left a comment

Choose a reason for hiding this comment

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

Crap, thanks for fixing this, this makes sense.

useAction = (action, options) => {
let useActionImpl: UseAction = createHookStub("useAction");

createHookStub("useAction", (adapter, coreHooks) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Dunno if you particularly care, but you could make createHookStub not actually do any variable rebinding in each module, and instead do it internally. Like:

const useAction: UseAction = createHookStub("useAction", (adapter, coreHooks) => {
  return (action, options) => {
    ...
  }
}

and then

const createHookStub = (factory) => {
  let resultFn;
  if (immediate) {
     resultFn = factory(....)
  } else {
     let impl;
     resultFn = (...args) => {
         if (impl) {
            return impl(...args);
        } else {
           throw
        }  
     }
     
     onGivenFactory(() => {
        impl = factory()
     });
  }
  return resultFn;
}

or some such

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yah that was one of the options that I explored with cursor; opus suggested the option in this PR was

My Recommendation: Pattern 1 (Internal Impl + Wrapper)
Pattern 1 is the cleanest and requires minimal changes:

@infiton infiton force-pushed the fix-self-referential-assignment branch from 5772e73 to 385ad9c Compare December 3, 2025 20:46
@infiton infiton force-pushed the fix-self-referential-assignment branch from 385ad9c to 856e877 Compare December 3, 2025 21:07
@infiton infiton merged commit 4b80c57 into main Dec 3, 2025
12 checks passed
@infiton infiton deleted the fix-self-referential-assignment branch December 3, 2025 21:19
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