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

Providing context to generated components in Solid doesn't work #49

Closed
4 tasks done
jsimonrichard opened this issue Dec 31, 2024 · 7 comments
Closed
4 tasks done
Labels
🤞 phase/open Post is being triaged manually

Comments

@jsimonrichard
Copy link

Initial checklist

Affected package

rehype-react@8.0.0

Steps to reproduce

Minimal reproducible example: https://codesandbox.io/p/devbox/mjnc4d
(no extra steps are required).

Actual behavior

It looks like a new reactive environment/context (whatever it's called) for Solid is being created based on this warning in browser's console: computations created outside a 'createRoot' or 'render' will never be disposed and the fact that context (using createContext) isn't passed down to components inserted through the components option of rehypeReact.

This is in contrast with rehype-react + React, with which it's possible to access context inside these inserted components.

Expected behavior

I expect the value of MyContext, set by MyContext.Provider, to be accessible inside interactive components inserted by rehype-react.

This is needed (I believe) for my use-case since I'm trying to render books (in asciidoc) with stateful, interactive content, and the content state needs to be saved on a per-user basis. Therefore I'm trying to use Context to pass user-specific data to the interactive components.

Runtime

node@20.12.0; Chrome (Brave): 1.73.89 Chromium: 131.0.6778.69 (Official Build) (64-bit)

Package manager

npm@10.5.0

Operating system

Arch Linux (kernel: 6.6.67-1-lts)

Build and bundle tools

Vite

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Dec 31, 2024
@wooorm
Copy link
Member

wooorm commented Jan 1, 2025

I dunno, Solid does things differently, I am not an export on what it does. If it does things wildly differently, perhaps that should be changed there?

I would typically recommend against context. Context is often not needed. Perhaps you can do something along these lines:

import {JSX, createSignal, onMount} from "solid-js";
import "./App.css";
import { unified } from "unified";
import rehypeParse from "rehype-parse";
import rehypeReact from "rehype-react";
import { Fragment, jsx, jsxs } from "solid-js/h/jsx-runtime";

function App() {
  const [content, setContent] = createSignal<JSX.Element>();

  onMount(async () => {
    const value = "Hello World"
    const processor = unified()
      .use(rehypeParse, { fragment: true })
      .use(rehypeReact, {
        Fragment,
        jsx,
        jsxs,
        elementAttributeNameCase: "html",
        stylePropertyNameCase: "css",
        components: {
          div: () => {
            return <div>Context: {value}</div>;
          },
        },
      });

    const file = await processor.process(`
<h1>Hello World</h1>
<div></div>
`);

    setContent(file.result);
  });

  return (
    <div>{content()}</div>
  );
}

export default App;

@jsimonrichard
Copy link
Author

Regardless of the use of context, doesn't the warning computations created outside a 'createRoot' or 'render' will never be disposed mean that using rehype-react with solid could lead to memory leaks?

Even if we can't really solve this right now, it might be worth putting a note in docs to acknowledge that this doesn't work as expected.

@wooorm
Copy link
Member

wooorm commented Jan 2, 2025

This tool supports some form of a contract, that contract is governed by React, the automatic JSX runtime. But all frameworks follow that contract, they say they do. Now, if one framework doesn’t support some of its features in some cases, to me that is out of scope to this project. It’s on Solid to support this JSX contract.

I would rather you raise this problem where it is, so that it can be fixed, instead of turning this readme into an issue tracker for Solid?
Is there a reason you think this does not need to be solved there?

@jsimonrichard
Copy link
Author

I'll raise this issue on the Solid side. I agree that this repo isn't the best place to solve this issue.

But because of the focused nature of this repo (in contrast with https://github.com/solidjs/solid) and because the instructions and examples for using rehype-react with Solid live here and in https://github.com/syntax-tree/hast-util-to-jsx-runtime, it makes sense to me to warn users of the current limitations here. They will probably run into these issues eventually, and that might prompt them to look at the solid issue tracker, but having a short warning in this repo's README might save them some time. If you'd like, we can also add a link to the issue I'll create on the solid side so that users know where to access the most up to date info.

@wooorm
Copy link
Member

wooorm commented Jan 3, 2025

instructions and examples […] live here

Few things live here? 😅
There are examples in hast-util-to-jsx-runtime, I am 👍 for a note in that example.

I understand that you want this project to work with Solid. But I don’t want to be an issue tracker for Solid.

Copy link

github-actions bot commented Jan 5, 2025

Hi! This was closed. Team: If this was fixed, please add phase/solved. Otherwise, please add one of the no/* labels.

@jsimonrichard
Copy link
Author

This solves the issue: solidjs/solid#2394 (comment)

SolidJS was unable to update the content correctly because using setContent(file.result) in an async context cannot be tracked by solid. Using setContent(() => file.result) instead, which brings file.result into a trackable context, fixes the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤞 phase/open Post is being triaged manually
Development

No branches or pull requests

2 participants