Skip to content

Feature/use effect cleanup delegate - #715

Merged
Freymaurer merged 4 commits into
mainfrom
feature/useEffect-cleanup-delegate
May 18, 2026
Merged

Feature/use effect cleanup delegate#715
Freymaurer merged 4 commits into
mainfrom
feature/useEffect-cleanup-delegate

Conversation

@Freymaurer

Copy link
Copy Markdown
Contributor

Update React.useEffect, React.useEffectOnce, React.useLayoutEffect and React.useLayoutEffectOnce to ensure proper cleanup function handling in case of empty body.

Closes #714

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes issue #714 where React.useEffect/useEffectOnce/useLayoutEffect/useLayoutEffectOnce, when given a body that only returns a cleanup function, was compiled by Fable so that the cleanup ran immediately during setup instead of being returned. The fix routes the unit -> (unit -> unit) overloads through a new System.Func<unit, (unit -> unit)> overload (and emit-based JS) so Fable cannot inline-invoke the returned cleanup.

Changes:

  • Add Func<unit, (unit -> unit)> based overloads of useEffect/useEffectOnce/useLayoutEffect/useLayoutEffectOnce and delegate the unit -> (unit -> unit) overloads to them.
  • Add tests (OnlyCleanup components) verifying that a body returning only a cleanup runs the cleanup on unmount; restructure existing tests into nested Components.UseEffect / UseEffectOnce / UseLayoutEffect / UseLayoutEffectOnce modules and split describe blocks per hook.
  • Bump CHANGELOG to 3.3.3 and modify the playground demo.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/Feliz/React/React.fs Adds Func-based overloads with explicit emitJsExpr; mass reformatting.
src/Feliz/CHANGELOG.md Adds 3.3.3 release note for #714.
tests/Feliz/ReactBindings/UseEffect.test.fs Refactors components into nested modules; adds OnlyCleanup regression tests.
tests/Feliz/ReactBindings/UseLayoutEffect.test.fs Same restructuring + OnlyCleanup regression tests for layout effect variants.
playground/src/Components.fs Replaces the lazy/Suspense demo with a small #714 reproduction; comments out LazyCounter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread playground/src/Components.fs
Comment thread src/Feliz/React/React.fs
Comment thread tests/Feliz/ReactBindings/UseLayoutEffect.test.fs Outdated
Comment thread src/Feliz/React/React.fs
Comment thread src/Feliz/React/React.fs
Comment thread tests/Feliz/ReactBindings/UseLayoutEffect.test.fs
…-once version, instead of replicating jsEmit
@Freymaurer
Freymaurer merged commit 0d20fe6 into main May 18, 2026
2 checks passed
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.

useEffect that only returns a cleanup function gets compiled incorrectly

2 participants