solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
32.26k stars 921 forks source link

Passing props to dynamic/injected children (conditionally?) #708

Closed kristianmandrup closed 2 years ago

kristianmandrup commented 2 years ago

Describe the bug

Not clear from docs how to pass attributes such as a ref to multiple (or conditional) children

import { render,  } from "solid-js/web";

function CounterContainer(props) {
  // how do I pass header into my Header child? how do I pass select props into all children?
  // using For? 

  return (
    <div class="container">
      {props.children}
    </div>
  );
}

function Header(props) {
  return <h2>{props.header}</h2>;
}

function App() {

  return (
    <>
      <CounterContainer header="my header"><Header /></CounterContainer>
    </>
  );
}

render(() => <App />, document.getElementById("app"));

Is it something like this?

  return (
    <div class="container">
      <For each={props.children}>
        {child => child({header: props.header)}
      </For>
    </div>
  );

How can I test what kind of child is being iterated, such as what tag? Can I call child as a function somehow?

  return (
    <div class="container">
      <For each={props.children}>
        {child => child.tag === 'Header' ? child({header: props.header)} child({...props}}
      </For>
    </div>
  );

Also not documented how to use Dynamic in the docs as far as I could see. Could I use Dynamic to achieve this?

  return (
    <div class="container">
      <For each={props.children}>
        {child => <Dynamic tag={child} {...props} />
      </For>
    </div>
  );

Using children in general is a bit of a mystery except for the simple cases. I looked through the spec files and couldn't find anything there either.

ryansolid commented 2 years ago

The children are already resolved at this point. Or would be using children helper. Basically children are a combination of arrays, signals, DOM nodes, strings, or anything else you might pass. But once resolved all signals will be reduced and it will just be the other options.

What I mean is unless you pass a function with arguments you can't call them or use Dynamic because it's the DOM node you are getting not the component constructor.

At which point all you can do is DOM manipulation. This fine for stuff like classes etc. But if you want to pass a ref it has to be before the children render. Your options are having your children be functions you call with data the comsumer can add to their element. This is the way For works. It is called renderProps in React if you want more reference. The other option is to use context.

Its possible I might not exactly be following the scenario. If you have an example even in a different framework I could better see what you are trying to do.

The main reason there isn't much on children is there isn't much in the way of specific patterns since there isn't much to be done with them. There is no VDOM. They are just the DOM Elements more or less. Better documentation on RenderProps would probably help though.

kristianmandrup commented 2 years ago

Thanks. I think it would greatly help with some kind of "Solid for React developers" that outlines some of the main differences, what you can do in a similar way, what and how you need to do certain things very differently.

kristianmandrup commented 2 years ago

In particular I'm trying to port reactstrap Accordion which can children such as AccordionItem

The Accordion can created as follows

  <Accordion>
    <AccordionItem>
      <AccordionHeader targetId="item-1">Item 1</AccordionHeader>  
      <AccordionBody accordionId="item-1"></AccordionBody>
    </AccordionItem>  
    <AccordionItem>
      <AccordionHeader targetId="item-2">Item 2</AccordionHeader>  
      <AccordionBody accordionId="item-2"></AccordionBody>
    </AccordionItem>  
</Accordion>

Here the Accordion render. Note that attributes may contain the special children prop. <Tag is a dynamic tag, for this I use <Dynamic in solid.

  return (
    <AccordionContext.Provider value={accordionContext}>
      <Tag {...attributes} className={classes} ref={innerRef} />
    </AccordionContext.Provider>
  );

So in this case, to pass down the className and ref to the child elements, I would have to use Context to pass them down?

mosheduminer commented 2 years ago

I think context is the best solution. The alternatives are to either 1) return a closure returning JSX (accepting some props), instead of returning JSX directly, and handle appropriately in the parent 2) have the children just return props, and write the JSX render in the parent (similar to how solid's <Switch>/<Match> is implemented. For whatever reason, despite dealing with this pattern a number of times in my react->solid ports, I never once thought of using context 🤔. But I think it's a good idea.

ryansolid commented 2 years ago

I don't see anything in this that you can't do the same way as React. It looks React is already using context. The one thing we can't do is React.cloneElement type shenanigans which I thought you were first talking about. This doesn't need anything fancy. Just use <Dynamic> in a similar way.

const Accordion = (props) => {
  const [local, context, attributes] = splitProps(props,
    ["flush", "className", "cssModule", "tag"],
    ["open", "toggle"]
  );

  const classes = () => mapToCssModules(classNames(
    local.className,
    'accordion',
    {
      'accordion-flush': local.flush
    }
  ), local.cssModule);

  return (
    <AccordionContext.Provider value={context}>
      <Dynamic {...attributes} tag={local.tag || "div"} className={classes()} />
    </AccordionContext.Provider>
  );
};
kristianmandrup commented 2 years ago

Hi @mosheduminer any chance you can help me out porting reactstrap, given that you have experience with similar ports? As part of porting reactstrap I've taken a stab at porting react-popper.

Most of the reactstrap components that are currently not yet working are using my buggy react-popper port.

mosheduminer commented 2 years ago

Hi @mosheduminer any chance you can help me out porting reactstrap, given that you have experience with similar ports?

At this point, I'm still gaining that experience 😂. I'm porting carbon-components, and there's quite a bit of work to do. I'm afraid I don't think I can split myself between two component libraries. I'm may be able to help on a small scale though, easiest way to reach me is on discord, if you're there.

kristianmandrup commented 2 years ago

What is splitProps? I'm learning a little more of the Solid particulars, going through the SolidJS tutorial... Found the part about avoiding destructuring of props

So I refactored Accordion to the following, but I guess that omit might "touch" the props to mess with reactivity, which could be where splitProps is useful? Also, looks like {props.children} might not be required after all, as Dynamic takes a children prop.

const defaultProps = {
  tag: 'div'
};

export const Accordion: Component = (props: PropTypes) => {
  const mprops = mergeProps({
    ...defaultProps,
    ...props
  });

  const classes = classname(
    mprops.className,
    'accordion',
    {
      'accordion-flush': mprops.flush
    }
  )    

  const [state, setState] = createStore({ open: mprops.open });
  const store = [
    state,
    {
      toggle() {
        setState(s => ({open: !s.open}));
      },
    },
  ];
  const attributes = omit(mprops, ['tag', 'innerRef'])

  return (
    <AccordionContext.Provider value={store}>
      <Dynamic component={mprops.tag} class={classes} {...attributes} ref={mprops.innerRef}>
        {props.children}
      </Dynamic>
    </AccordionContext.Provider>
  );
};

Ah, here the splitProps is mentioned... :)

kristianmandrup commented 2 years ago

So Accordion currently look like this with my current slightly better understanding

const defaultProps = {
  tag: 'div'
};

export const Accordion: Component = (props: PropTypes) => {
  const mprops = mergeProps({
    ...defaultProps,
    ...props
  });

  const [local, attributes] = splitProps(mprops,
    ["flush", "open", "toggle", "className", "tag", "innerRef"],
  );

  const classes = classname(
    local.className,
    'accordion',
    {
      'accordion-flush': local.flush
    }
  )    

  const [state, setState] = createStore({ open: local.open });
  const store = [
    state,
    {
      toggle() {
        setState(s => ({open: !s.open}));
      },
    },
  ];

  return (
    <AccordionContext.Provider value={store}>
      <Dynamic component={local.tag} class={classes} {...attributes} ref={local.innerRef}/>
    </AccordionContext.Provider>
  );
};

Obviously I could and should replace createStore with a simple createSignal

ryansolid commented 2 years ago

Not sure why you are making local state for toggle. Also spreading like that with mergeProps doesn't work. Pass separate arguments.

Have you tried what I wrote above? I renamed innerRef to ref because it's better for Solid's automatic compilation handling. But that should roughly be the whole component.

kristianmandrup commented 2 years ago

OK, I will try your suggestion next. Have just gone through the full tutorial. I would suggest adding links to the Tutorial pages in the main SolidJS docs. Wasn't clear where to find the detailed info/examples. Perhaps at the top of the docs make a point that the docs are just scratching the surface and to go more in-depth, go through the tutorial. Just some thoughts from my journey so far.

I'm starting to see your point regarding toggle. I went down the wrong path when I initially started out. I think your idea of just passing in the context from outside also makes more sense instead of setting up an internal one. You are correct that the toggle function is to be provided by the consumer.

  const accordionContext = useMemo(() => ({
    open,
    toggle,
  }));

Thanks for all the assistance and advise. Slowly getting there ;)