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

Add Fragment to @stencil/core #1151

Closed
NEO97online opened this issue Oct 16, 2018 · 17 comments
Closed

Add Fragment to @stencil/core #1151

NEO97online opened this issue Oct 16, 2018 · 17 comments
Labels
ionitron: stale issue This issue has not seen any activity for a long period of time

Comments

@NEO97online
Copy link

NEO97online commented Oct 16, 2018

Stencil version:

 @stencil/core@0.13.2

I'm submitting a:

[ ] bug report
[x] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://stencil-worldwide.herokuapp.com/ or https://forum.ionicframework.com/

Current behavior:
Rendering multiple elements requires either a wrapping div or returning an array. Returning an array avoids rendering an unwanted <div>, but array format with JSX is hard to read and work with, especially in larger components.

Expected behavior:
Just like React, you could wrap multiple elements in a <Fragment> tag.

Steps to reproduce:
Without fragment:

render() {
  return [
    <div>My first element</div>,
    <div>
        <span>Another element</span>
    </div>,
    <div>And another</div>
  ]
}

Related code:
With fragment:

render() {
  return (
    <Fragment>
      <div>My first element</div>
      <div>
        <span>Another element</span>
      </div>
      <div>And another</div>
    </Fragment>
  )
}

Other information:

This is an extremely simple feature to add, I'm just not sure where it should go in the stencil repository.
The working Fragment code is literally just this one line:

export const Fragment = (props, children) => children

Or without ES6:

export function Fragment(props, children) {
  return children
}

Used as a JSX element, this simply renders the children without rendering an arbitrary wrapping element.

I made a package here to provide this feature, but it would be fantastic if it were available in @stencil/core. People coming from React will love this.

If someone can advise me where in the repo to add this code, I would be happy to make the PR myself.

@ionitron-bot ionitron-bot bot added the triage label Oct 16, 2018
@simonhaenisch

This comment has been minimized.

@NEO97online
Copy link
Author

@simonhaenisch Yeah, this isn't a web component, it's just a JSX tag! So, no dash.

@manucorporat
Copy link
Contributor

@michaelauderer we would like to be able to use the short-syntax:

<>

</>

but looks like it's not supported by typescript outside React, "React" is hardcoded into typescript: microsoft/TypeScript#20469

@NEO97online
Copy link
Author

@manucorporat Yes, this would be even nicer, but until we get support from typescript it would be great to at least include the Fragment element. <Fragment> is still commonly used in React anyways so it would make sense to have this. It could be a long time before Typescript supports the short-syntax.

@o-t-w
Copy link

o-t-w commented Feb 20, 2019

Is there any update on this? Not having full control over markup is a pretty drastic limitation. Particularly now with CSS grid, markup structure matters for layout and I can't just have unnecessary divs around everything. Pretty sure LitElement doesn't have this drawback.

@simonhaenisch
Copy link
Contributor

simonhaenisch commented Feb 20, 2019

@o-t-w not sure what you mean because this doesn't limit your ability to have full control over your markup? It's just syntactic sugar (that indeed increases readability a lot), i. e. you can already either use arrays or add the suggested Fragment functional component into your project.

@NEO97online
Copy link
Author

@o-t-w For now, I've already built a solution for it here: https://github.com/michaelauderer/stencil-fragment

If someone can point out where it belongs in the Stencil repository, I'll make a PR. (@manucorporat, any thoughts?)

@manucorporat
Copy link
Contributor

Stencil 1.0 introduces a new <Host> exported from @stencil/core that can act as a Fragment!

@liron-navon
Copy link

@manucorporat
I found this super confusing,<Host> is nothing like react <Fragment>,

Error: The <Host> must be the single root component. Make sure:
- You are NOT using hostData() and <Host> in the same component.
- <Host> is used once, and it's the single root component of the render() function.

@sfmskywalker
Copy link

I, too would like to see<Fragment> be part of StencilJS. My use case is that I have a modular web component where consumers of the component can declare plugins, tat themselves render widgets. These renderings are either JSX (Vdom objects) or string values (in case the consumer is a vanilla JS app with no JSX support). When the widget is rendered as string, I need to set the innerHTML property of a containing element. Ideally, without having to use a <div>.

Kudos to @auderer for providing exactly that!

@sfmskywalker
Copy link

Looks like I celebrated too soon. I misunderstood Fragment to also have an InnerHTML property somehow causing that content to magically be rendered. Instead, it looks like it's just a container that renders child elements. It makes sense that it doesn't do what I expected with InnerHTML.

@elpupi
Copy link

elpupi commented Jun 21, 2020

As @auderer said and did in his Github project, it is just a one-liner:

export default (props, children) => [ ...children ]

Why wouldn't be possible to add it in @stencil/core?

@simonhaenisch
Copy link
Contributor

Actually, microsoft/TypeScript#38720 was just merged so we can use the <></> syntax soon!

@elpupi
Copy link

elpupi commented Jun 26, 2020

Ok. Good news. But in the meanwhile, it would be nice to have a Fragment functional component (without the syntactic sugar)

@simonhaenisch
Copy link
Contributor

In the meanwhile you could also just add your one-liner into your project(s) 😬 Feel free to open a PR though, but no promises that it'll be merged (up to the maintainers). Especially with a better solution in sight already.

@elpupi
Copy link

elpupi commented Jun 27, 2020

No worries. I wasn't going to wait before an action on the codeline would be made. I know how slow an organization is :) On top of that, majority of issues are closed as soon as possible even if the issue exists. We'll see what's happening with the new <>..<> TS syntax support.

@ionitron-bot
Copy link

ionitron-bot bot commented Jul 27, 2020

Thanks for the issue! This issue is being closed due to inactivity. If this is still an issue with the latest version of Stencil, please create a new issue and ensure the template is fully filled out.

Thank you for using Stencil!

@ionitron-bot ionitron-bot bot added the ionitron: stale issue This issue has not seen any activity for a long period of time label Jul 27, 2020
@ionitron-bot ionitron-bot bot closed this as completed Jul 27, 2020
@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Jul 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ionitron: stale issue This issue has not seen any activity for a long period of time
Projects
None yet
Development

No branches or pull requests

7 participants