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

Show double evaluation #2411

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

TPReal
Copy link

@TPReal TPReal commented Jan 24, 2025

Summary

This change removes unnecessary evaluations of the when conditions in the <Show> and <Match> components. Both these components used to evaluate the condition twice immediately after creation if the condition was true, children was specified as a function and keyed was not specified.

This was especially problematic when the condition produced JSX.Element which was then displayed by the children function, as two copies of the elements were instantiated.

This change also removes unnecessary evaluations of <Match> conditions in a sutiation where the condition did not change, but only a later <Match>'s condition changed.

How did you test this change?

Added unit tests demonstrating the described problems, and made pnpm test pass.

fixes #2406

The `<Show>` component evaluates its `when` condition more often than necessary, in particular it is immediately evaluated twice if the condition is true, children is specified as a function, and keyed is not specified.

solidjs#2406
Adds another memo directly on `when` in `<Show>`.

solidjs#2406
This removes a bug where the `when` condition of a `<Match>` was evaluated twice immediately after creation, when the condition was true, children was a function and keyed was not specified. It also removes any unnecessary conditions evaluations by creating a memo on every `when` in a `Switch`.

For example, if a `<Switch>` has two `<Match>`es with `when={a()}` and `when={b()}` respectively, then:
- `b()` is never called if `a()` is truthy (which was true also before this change),
- `a()` is never called when `b()` changes (which is new).

solidjs#2406
Copy link

changeset-bot bot commented Jan 24, 2025

🦋 Changeset detected

Latest commit: 2174599

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
solid-js Patch
test-integration Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to 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.

The condition of <Show> runs another time unnecessarily when using the function form
1 participant