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

[Bug?]: hydration mismatch on ssr, and nested ternaries in jsx with objects or signals #2100

Closed
2 tasks done
birkskyum opened this issue Mar 13, 2024 · 11 comments
Closed
2 tasks done
Labels
bug Something isn't working

Comments

@birkskyum
Copy link
Contributor

birkskyum commented Mar 13, 2024

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

With SSR, this gives a hydration error:

export default function App() {
  const obj1 = {
    prop: true,
  };

  const obj2 = {
    prop: true,
  };

  return <>{obj1.prop ? obj2.prop ? <div>Output</div> : <></> : <></>}</>;
}

The error also exist with signals instead of plain objects

import { createSignal } from 'solid-js';

export default function App() {
  const [signal1, setSignal1] = createSignal(true);
  const [signal2, setSignal2] = createSignal(true);

  return <>{signal1() ? signal2() ? <div>Output</div> : <></> : <></>}</>;
}

The error is:

Uncaught Error: Hydration Mismatch. Unable to find DOM nodes for hydration key: 1
    at getNextElement (chunk-7APIXOX5.js?v=fd41909e:275:37)
    at app.tsx:12:25
    at normalizeIncomingArray (chunk-7APIXOX5.js?v=fd41909e:486:51)
    at insertExpression (chunk-7APIXOX5.js?v=fd41909e:438:9)

Expected behavior 🤔

I expected this to work, like it does with ssr:false.

The code also runs fine if I replace this:

return <>{obj1.prop ? obj2.prop ? <div>Output</div> : <></> : <></>}</>;

With anything else really..

return <div>{obj1.prop ? obj2.prop ? <div>Success</div> : <></> : <></>}</div>;
return <>{true ? obj2.prop ? <div>Success</div> : <></> : <></>}</>;
return <>{obj1.prop ? true ? <div>Success</div> : <></> : <></>}</>;
return <>{obj1 ? obj2 ? <div>Success</div> : <></> : <></>}</>;
return <>{obj1.prop && obj2.prop && <div>Success</div>}

Steps to reproduce 🕹

Steps:

  1. Take the Bare example, and update the App component
  2. Look in console.

Here is a stackblitz:

Context 🔦

No response

Your environment 🌎

No response

@birkskyum birkskyum added the bug Something isn't working label Mar 13, 2024
@birkskyum birkskyum changed the title [Bug?]: hydration issue on ssr [Bug?]: hydration mismatch on ssr, and double ternaries in jsx Mar 13, 2024
@birkskyum birkskyum changed the title [Bug?]: hydration mismatch on ssr, and double ternaries in jsx [Bug?]: hydration mismatch on ssr, and nested ternaries in jsx with objects Mar 13, 2024
@birkskyum birkskyum changed the title [Bug?]: hydration mismatch on ssr, and nested ternaries in jsx with objects [Bug?]: hydration mismatch on ssr, and nested ternaries in jsx with objects/signals Mar 13, 2024
@birkskyum birkskyum changed the title [Bug?]: hydration mismatch on ssr, and nested ternaries in jsx with objects/signals [Bug?]: hydration mismatch on ssr, and nested ternaries in jsx with objects Mar 13, 2024
@birkskyum birkskyum changed the title [Bug?]: hydration mismatch on ssr, and nested ternaries in jsx with objects [Bug?]: hydration mismatch on ssr, and nested ternaries in jsx with objects or signals Mar 13, 2024
@ryansolid
Copy link
Member

I have no idea how you narrowed this down. But I'm impressed.

@lxsmnsyc
Copy link
Member

lxsmnsyc commented Mar 14, 2024

Is this expected

image

edit:
nvm, it shows up in the console, lol

@ryansolid
Copy link
Member

I'm going to move this to Solid.. I'm about 99% sure this is a compilation mismatch error.

@ryansolid ryansolid transferred this issue from solidjs/solid-start Mar 14, 2024
@birkskyum
Copy link
Contributor Author

birkskyum commented Mar 16, 2024

I can repro here:

I can't repro here:

@birkskyum
Copy link
Contributor Author

birkskyum commented Mar 16, 2024

I think I've figured why the error doesn't repro in the solid-ssr examples as i expected: It only shows up in dev mode, and they only have build/start.

For both astro, vinxi, solid-start I don't see the error when previewing a production build.

@lxsmnsyc
Copy link
Member

@birkskyum the only common thing between the three is that they are in SSR and they use the Vite plugin.

@birkskyum
Copy link
Contributor Author

birkskyum commented Mar 16, 2024

Alright, if I remove the dev-only flag in dom-expressions for the Hydration Mismatch error, then I can repro with the solid-ssr example as well - without the vite-plugin-solid. It's a bit easier to debug.

@birkskyum
Copy link
Contributor Author

birkskyum commented Mar 16, 2024

This appear to be a bug in babel-plugin-jsx-dom-expressions

@birkskyum
Copy link
Contributor Author

birkskyum commented Mar 17, 2024

function Comp (){
  const obj1 = {
    prop: true,
  };

  const obj2 = {
    prop: true,
  };

  return <>{obj1.prop ? obj2.prop ? <span>4</span> : <></> : <></>}</>;
}

Compiles to this:

function App() {
  const obj1 = {
    prop: true
  };
  const obj2 = {
    prop: true
  };
  return _$memo((() => {
    const _c$ = _$memo(() => !!obj1.prop);
    return () => _c$() ? (() => {
      const _c$2 = _$memo(() => !!obj2.prop);
      
--    // Current / Wrong
--    return () => _c$2() ? _$getNextElement(_tmpl$) : [];
      
++     // Correct
++    return _c$2() ? _$getNextElement(_tmpl$) : [];
          
    })() : [];
  })());
}

or potentially less aggressively memoized

function App() {
  const obj1 = {
    prop: true
  };
  const obj2 = {
    prop: true
  };

  return [obj1.prop ? obj2.prop ? _$getNextElement(_tmpl$) : [] : []]
}

@birkskyum
Copy link
Contributor Author

birkskyum commented Mar 17, 2024

Explanation

Seems like the wrapping memo is lacking a "() =>", and every memo inside has an extra.

Therefore (inline syntax for clarity)

0 x obj1.prop - this works

<>{_$getNextElement(_tmpl$)}</>

Result

return getNextElement(_tmpl$);

1 x obj.prop - this works, accidentally

<>{ obj1.prop ? _$getNextElement(_tmpl$) : [] }</>

Result

return createMemo( // Missing "() =>" from the wrapper

    () => (createMemo(() => !!obj1.prop)() ? getNextElement(_tmpl$) : [])  // inserted per obj1.prop

);

here we see the wrapper isn't a function, but fortunately there is an extra () => inside to counter that, and the result is this does work:

return createMemo(() => // Rely on the "() =>" from below line

 (createMemo(() => !!obj1.prop)() ? getNextElement(_tmpl$) : []) 

);

2 x obj.prop

<>{ obj1.prop ? obj1.prop ? _$getNextElement(_tmpl$) : [] : [] }</>

Result

return createMemo( // Missing "() =>" from the wrapper

    () => (createMemo(() => !!obj1.prop)() ? 

       () => (createMemo(() => !!obj1.prop)() ? getNextElement(_tmpl$) : [])  // inserted per obj1.prop
//      ^ this "()=>" should not be here as the function is never invokes. the effect is that "getNextElement" never get's called

   : []) 

);

this is where things start to go sideways. The outermost memo still accidentally pairs with the () => inside, but there is an extra one added from the nesting which is never called, and that will create the hydration mismatch.

It only get's worse when nesting further, as an extra uncalled () => will be made each layer.

Solution

an extra () => should be added to the outermost wrapper, and no () => should wrap each memo inside.

Result

-- return createMemo(
++ return createMemo(() =>

--       () => (createMemo(() => !!obj1.prop)() ? getNextElement(_tmpl$) : [])  // inserted per obj1.prop
++      (createMemo(() => !!obj1.prop)() ? getNextElement(_tmpl$) : [])  // inserted per obj1.prop


);

Outcome

Now every added wrapping of obj1.prop ? : [] corresponds to simply replacing:

getNextElement(_tmpl$)

with the green line above:

(createMemo(() => !!obj1.prop)() ? getNextElement(_tmpl$) : []) 

@ryansolid
Copy link
Member

Hmm.. I wonder if I did this for a reason. But didn't reflect it in SSR. The one thing that returning the function does is isolate the execution.. however returning a non-memoized function seems weird. So this is probably my mistake. There was a compilation issue where we were hoisting up the memo outside of the prop getter. I changed the behavior then. I'd love to track that down.

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

Successfully merging a pull request may close this issue.

3 participants