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?]: Cannot access context when read in Meta content prop. #41

Open
thetarnav opened this issue Nov 7, 2023 · 4 comments
Open

[Bug?]: Cannot access context when read in Meta content prop. #41

thetarnav opened this issue Nov 7, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@thetarnav
Copy link

thetarnav commented Nov 7, 2023

If you try to read context inside content prop of Meta component provided by start, the context will not be available, which is not expected as it's used directly under the provider.

<Ctx.Provider value={{ text: 'you should see this' }}>
  <Html lang="en">
    <Head>
      <Meta
        name="description"
        content={useContext(Ctx).text} // ctx read here is not available
      />
    </Head>
    <Body>...</Body>
  </Html>
</Ctx.Provider>

https://stackblitz.com/edit/solid-start-meta-context-issue?file=src%2Froot.tsx

I'm interested in working on a fix if this is something that's considered a bug.

@thetarnav thetarnav added the bug Something isn't working label Nov 7, 2023
@pucho
Copy link

pucho commented Nov 13, 2023

Just to add my 2c, my experience is regarding Context on React.

That said, in your example you don't define a default state for your context e.g:

const Ctx = solid.createContext<{ text: string }>({text: 'default ctx'});

With that change the meta tag ends up with it's description being "default ctx", if I had to take a stab at it, at the moment you try to render the root component, the only available value for your ctx is the one on the initial definition.

If you were to move all the Head related stuff to a new component like so:

import * as solid from 'solid-js';
import { Head, Meta, Title } from 'solid-start';
import { Ctx } from './root';

export default function Header() {
  const value = solid.useContext(Ctx);
  return (
    <Head>
      <Title>SolidStart - Bare</Title>
      <Meta charset="utf-8" />
      <Meta name="viewport" content="width=device-width, initial-scale=1" />
      <Meta name="description" content={value?.text} />
    </Head>
  );
}

You'll be able to deal with the context in a better way, your approach feels more like an anti-pattern than a bug.

@thetarnav
Copy link
Author

thetarnav commented Nov 13, 2023

I agree that reading context anywhere but in component body is usually a bad practice.
But I have a good reason for this, I'm working on a i18n library that will access context every time you call m.greeting({ name: "John" }) etc, which will be used mostly in JSX, also in <head>. (It needs to read from ctx to avoid state leaking on the server)

Making it into a separate component solves the problem of course, so is inlining one: https://github.com/thetarnav/paraglide-solidstart-hackernews/blob/main/src/root.tsx#L25

I'm considering this a bug because reading ctx in any other prop will work fine:

<Ctx.Provider value={...}>
	<div class={useContext(Ctx)} />
</Ctx.Provider>

thats because props are being read lazily in a computation, which will have access to the context when executed. Meta should be working the same way, since it is a core start component, that's meant to match the native <meta> tag.
Also resolving children works differently in solid compared to react. That code would be invalid in React, while in Solid it depends on how well the component is implemented.
That same template when ported to react would look more like this:

<Ctx.Provider value={{ text: 'you should see this' }}>
  {() => <Html lang="en">
    {() => <>
    	<Head>
      	{() => <Meta name="description" content={() => useContext(Ctx).text} />}
    	</Head>
    	<Body>...</Body>
    </>}
  </Html>}
</Ctx.Provider>

because props are compiled into getters and read lazily, so one could argue that there are many additional components between the provider and the ctx read.

@ryansolid
Copy link
Member

Yeah.. it's because of the lazy access of meta tags to insert them in the head. I see its in the head there but we don't really need to render them until we collect them all. I guess this is a solid meta issue. I will move there.

@aguilera51284
Copy link

any solutions for this?

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

No branches or pull requests

4 participants