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

The component is initialized inexplicably #2402

Open
Reinhard2019 opened this issue Jan 10, 2025 · 6 comments
Open

The component is initialized inexplicably #2402

Reinhard2019 opened this issue Jan 10, 2025 · 6 comments
Labels
bug Something isn't working compilation

Comments

@Reinhard2019
Copy link

Describe the bug

I have a Tooltip component, when clicked, it should show the content, but instead of showing the content, it reinitialized. My original code was quite complex, and I spent most of the day looking for specific bug causes. I ended up with a minimal example that could be reproduce this bug.

Your Example Website or App

https://playground.solidjs.com/anonymous/de1b3aef-e779-4949-bc37-9f7762549216

Steps to Reproduce the Bug or Issue

  1. go to https://playground.solidjs.com/anonymous/de1b3aef-e779-4949-bc37-9f7762549216
  2. click text

Expected behavior

Once the tooltip is clicked, the content should be displayed instead of reinitialized.

Screenshots or Videos

截屏2025-01-10 10 10 08

Platform

  • OS: [e.g. macOS, Windows, Linux]
  • Browser: [e.g. Chrome, Safari, Firefox]
  • Version: [e.g. 91.1]

Additional context

No response

@hixb
Copy link

hixb commented Jan 10, 2025

Problem Explanation:

The issue arises because of how Solid.js handles conditional rendering. In your code, the Tooltip component is conditionally rendered using this logic:

{exp1() ? null : exp2() ? <Tooltip /> : null}

Whenever the conditions for rendering the Tooltip change, Solid.js destroys the existing Tooltip instance and creates a new one. This is why you see console.log("Tooltip init") being logged repeatedly, indicating that the component is being re-initialized unnecessarily.

Root Cause:

Solid.js reactivity system ensures that components are completely destroyed and recreated when conditional rendering ({} or <Show />) evaluates to a different truthy or falsy value. This is intentional behavior to allow full control over the component lifecycle. However, in this case, the component should not be destroyed—only its visibility should be toggled.

Solution:

To prevent the Tooltip component from being re-initialized, can use the <Show /> component to wrap the Tooltip. This ensures the Tooltip itself is instantiated only once and its visibility is controlled by a reactive signal:

function App() {
  return (
    <div>
      <Show when={!exp1() && exp2()}>
        <Tooltip />
      </Show>
    </div>
  );
}

With this change:

  1. The Tooltip component will only initialize once, even if the conditions for showing or hiding it change.
  2. Visibility toggling is handled more efficiently without destroying and recreating the component.

@Reinhard2019
Copy link
Author

Problem Explanation:

The issue arises because of how Solid.js handles conditional rendering. In your code, the Tooltip component is conditionally rendered using this logic:

{exp1() ? null : exp2() ? <Tooltip /> : null}

Whenever the conditions for rendering the Tooltip change, Solid.js destroys the existing Tooltip instance and creates a new one. This is why you see console.log("Tooltip init") being logged repeatedly, indicating that the component is being re-initialized unnecessarily.

Root Cause:

Solid.js reactivity system ensures that components are completely destroyed and recreated when conditional rendering ({} or <Show />) evaluates to a different truthy or falsy value. This is intentional behavior to allow full control over the component lifecycle. However, in this case, the component should not be destroyed—only its visibility should be toggled.

Solution:

To prevent the Tooltip component from being re-initialized, can use the <Show /> component to wrap the Tooltip. This ensures the Tooltip itself is instantiated only once and its visibility is controlled by a reactive signal:

function App() {
  return (
    <div>
      <Show when={!exp1() && exp2()}>
        <Tooltip />
      </Show>
    </div>
  );
}

With this change:

  1. The Tooltip component will only initialize once, even if the conditions for showing or hiding it change.
  2. Visibility toggling is handled more efficiently without destroying and recreating the component.

First of all, thank you for your reply, I know there are many ways to get around this bug. But my concern is that if this problem persists, I may not dare to use the ternary operator arbitrarily in the future, and there is a problem with using the Show component, I am also using ts, and the Show component can't filter out the extra ts types based on the judgment criteria.

@ryansolid
Copy link
Member

Yeah something odd is going on. The commented out code potentially causing an issue is expected, but the compiler shouldn't be returning functions in functions that aren't memoized. It doesn't appear to be as the iife's call themselves. And yet this strange behavior. I will step through it when I get a moment.

@ryansolid
Copy link
Member

ryansolid commented Jan 10, 2025

Yeah confirmed this is a compilation bug. Nested expressions shouldn't be returning a function and just inlining the memo. Thanks for reporting I'm surprised no one noticed this earlier. I'm positive this wasn't always the case but with all the nested iife's sometimes it's hard to see what is going on and might have gone under the radar when we made other changes.

The output should be:

var _el$3 = _tmpl$3();
_$insert(_el$3, (() => {
      var _c$ = _$memo(() => !!exp1());
      return () => _c$() ? null : (() => {
        var _c$2 = _$memo(() => !!exp2());
        return _c$2() ? _$createComponent(Tooltip, {}) : null;
      })();
    })());
return _el$3;

while we can hoist the first conditional we can't hoist the rest of them so we should just inline them. I should talk about this on stream sometime.

@ryansolid ryansolid added bug Something isn't working compilation labels Jan 10, 2025
@mdynnl
Copy link
Contributor

mdynnl commented Jan 10, 2025

I think we already have a better output done for component props https://playground.solidjs.com/anonymous/04bc4f1a-ddba-4e9f-9973-7fff240bb940 which directly invokes the memo signal. The only thing the current output does better is the first conditional gets hoisted.

component props output is like this

_$createComponent(App2, {
  get children() {
    return _$memo(() => !!exp1())() ? null : _$memo(() => !!exp2())() ? _$createComponent(Tooltip, {}) : null;
  }
})

@ryansolid
Copy link
Member

@mdynnl I agree. I think the hoisting is a nice touch that benefits many cases but except for the top level we should be doing it like the component I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compilation
Projects
None yet
Development

No branches or pull requests

4 participants