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

Special syntax for child functions #104

Closed
Lenne231 opened this issue Jan 10, 2018 · 8 comments
Closed

Special syntax for child functions #104

Lenne231 opened this issue Jan 10, 2018 · 8 comments
Labels
Proposal Proposals (haven't confirmed)

Comments

@Lenne231
Copy link

It would be nice to have a special syntax for child functions.

i.e.

<ThemeContext.Consumer>
  <(theme)>
    <h1 style={{color: theme === 'light' ? '#000' : '#fff'}}>
      {this.props.children}
    </h1>
  </>
</ThemeContext.Consumer>

instead of

<ThemeContext.Consumer>
  {theme => (
    <h1 style={{color: theme === 'light' ? '#000' : '#fff'}}>
      {this.props.children}
    </h1>
  )}
</ThemeContext.Consumer>
@TrySound
Copy link

TrySound commented Jan 10, 2018

@Lenne231 "Nice" is not the reason to add the feature. Your proposal doesn't have any benefits over current solution, but will introduce more work for analyzers.

@syranide
Copy link
Contributor

The angle brackets should logically indicate that it creates and returns an element, but it actually returns a function.

@kastigar
Copy link

kastigar commented Feb 2, 2018

And what about this? Lets take react-toggled example:

<Toggle>
    {({on, getTogglerProps}) => (
      <div>
        <button {...getTogglerProps()}>Toggle me</button>
        <div>{on ? 'Toggled On' : 'Toggled Off'}</div>
      </div>
    )}
</Toggle>

And replace it with:

<Toggle({on, getTogglerProps})>
   <div>
    <button {...getTogglerProps()}>Toggle me</button>
    <div>{on ? 'Toggled On' : 'Toggled Off'}</div>
  </div>
</Toggle>

Or above example:

<ThemeContext.Consumer(theme)>
    <h1 style={{color: theme === 'light' ? '#000' : '#fff'}}>
      {this.props.children}
    </h1>
</ThemeContext.Consumer>

It is similar to function declaration:

function Foo(x, y) { return x + y; }
<Foo(x, y)>{x+y}</Foo>

and can be read as "This element receives arguments x and y and can use them in children".

More examples:

<For(item) of={list}>
  <li key={item.id}>{item.name}</li>
</For>;

<Motion(interpolatingStyle) defaultStyle={{x: 0}} style={{x: spring(10)}}>
  <div style={interpolatingStyle} />
</Motion>;

<PinchZoomPan(x, y, scale) width={width} height={height}>
  <img
    src={`https://placekitten.com/${width}/${height}`}
    style={{
      pointerEvents: scale === 1 ? 'auto' : 'none',
      transform: `translate3d(${x}px, ${y}px, 0) scale(${scale})`,
      transformOrigin: '0 0',
    }}
  />
</PinchZoomPan>

@syranide
Copy link
Contributor

syranide commented Feb 2, 2018

@kastigar What's wrong with:

<Toggle callback={({on, getTogglerProps}) => (
  <div>
    <button {...getTogglerProps()}>Toggle me</button>
    <div>{on ? 'Toggled On' : 'Toggled Off'}</div>
  </div>
)} />

Uses the existing flexibility and doesn't require any new syntax. It also allows you to trivially move the function body somewhere else if the code gets unwieldy at some point.

@kastigar
Copy link

kastigar commented Feb 4, 2018

@syranide It doesn't look naturally. jsx is XML-like syntax. And rener prop "as-is" is alike semi-solution IMHO.

<PinchZoomPan(x, y, scale) width={width} height={height}>
  <img ... />
</PinchZoomPan>

In above snippet img is child. As expected. With children as render prop it is not.
children as render prop becomes more and more popular. I think it is time to make it more native for jsx.

P.S. I don't insist that my or @Lenne231 solution is the best.

@syranide
Copy link
Contributor

syranide commented Feb 4, 2018

It doesn't look naturally. jsx is XML-like syntax.

Engineering/programming is about the correct solution for the problem, not a pleasing aesthetic.

And rener prop "as-is" is alike semi-solution IMHO.

It's an intended feature, just as JSX-children is just sugar for the children property. If you have a component that takes two sets of children, the correct engineering solution is to do:

<MyComp
 childrenA={<div />}
 childrenB={<div />}
/>

Whether it's aesthetically pleasing or not a secondary concern, that doesn't mean it's wrong to consider extending the syntax like you want. However, there's has to be a very strong use-case for it; that style of component you're using cannot be found anywhere in my projects. So in my case it would be entirely useless and that's a concern for adding a new syntax, how small/big is the actual the audience? Could it even encourage bad practices?

Please don't take my response as offensive, I'm simply trying to explain the problem from another perspective for your benefit. So I think if you want others to take this proposal seriously, you also need a convincing argument for why it's a really good idea for a wide audience that makes it worth the effort and practical/technical costs. Because that is not obvious to me.

@dantman
Copy link

dantman commented Mar 4, 2018

I have a problem with the suggested expansion of:

<Foo(arg)>
   ...
</Foo>

IMHO this does not look like it would expand to the proposed:

<Foo>
    {(arg) => (
      "..."
    )}
</Foo>

To me, it looks like it would expand to:

const _Foo = Foo(arg);
<_Foo>
   ...
</_Foo>

Which would make more logical sense. And might even somewhat make sense in some situations.

class MyComponent extends HTMLElement {
  static get is() { return 'my-component'; }
  // ...
}
customElements.define(MyComponent.is, MyComponent);

const wc = (WebComponent) => WebComponent.is;

const ReactComponent = (props) => {
  <div>
    <wc(MyComponent)>
      ...
    </wc>
  </div>
};

// Or perhaps, if you want to be really strict

const ReactComponent = (props) => {
  <div>
    <wc(MyComponent)>
      ...
    </wc(MyComponent)>
  </div>
};

So on the chance that someone comes up with a better use case for something like that. I would definitely oppose co-opting that more logical usage of this syntax to instead refer to the proposed as-render prop expansion.

@Huxpro Huxpro added the Proposal Proposals (haven't confirmed) label Feb 25, 2022
@Huxpro
Copy link
Contributor

Huxpro commented Feb 25, 2022

Personally I felt like we wouldn't pursue in this direction. Feel free to reopen if any one has a strong second opinion on this.

@Huxpro Huxpro closed this as completed Feb 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposals (haven't confirmed)
Projects
None yet
Development

No branches or pull requests

6 participants