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 helper functions for fragments #319

Closed
nbardy opened this issue Oct 31, 2017 · 11 comments
Closed

Add helper functions for fragments #319

nbardy opened this issue Oct 31, 2017 · 11 comments
Milestone

Comments

@nbardy
Copy link
Contributor

nbardy commented Oct 31, 2017

https://reactjs.org/docs/react-component.html#fragments

Add support for fragments.

e.g.

(defn sample-component [coll]
 (for [v coll]
  [:li v]))

Should return a fragment when called with [sample-component (range 6)].

@Deraen
Copy link
Member

Deraen commented Oct 31, 2017

Could you try (apply array (for ...))? Looks like fragments just need to be JS Arrays.

There is also createFrament addon: https://reactjs.org/docs/create-fragment.html

I tried checking React code if there is way to extend React to understand other data-structures as fragments (though that might anyway be bad idea) but didn't find anything. And it might be hard in Reagent to automatically detect which returns values are fragments. Only seq? or all sequential? (i.e. vector)? If vector is allowed, we'd need to detect between elements, probably checking if the first item is fn or keyword. Might be good solution to add fragments function which just converts sequential collection to JS array.

@nbardy
Copy link
Contributor Author

nbardy commented Nov 2, 2017

Hmm. The array solution is nice. It would also be nice to have native cljs support. I chooseseq as my initial idea since reagent supports the seqs as body of an element convention. It would be nice if seqs were treating as fragments in that case. I always found it a little unintuitive how they are unfurled into the parent. But that's definitely not worth it to break backwards compatibility. A fragments function seems like a good approach.

@Deraen
Copy link
Member

Deraen commented Nov 6, 2017

This one is useful for e.g. conditionally creating 1 or 2 elements:

(defn fragments [& children]
  (apply array (map r/as-element (keep identity children))))
(fragments
  ^{:key 1} [:div "foo"]
  (if open?
    ^{:key 2} [:div "hello!"]))

Do we need another, non-vargs, version? For cases when using for etc.
Or could be do with only the non-vargs version?

@Deraen Deraen changed the title Add support for fragments Add helper functions for fragments Nov 28, 2017
@Deraen Deraen added this to the 0.9.0 milestone Nov 28, 2017
@minikomi
Copy link

minikomi commented Nov 29, 2017

Could something like

(defn <> [& children]
   (apply array (map r/as-element (keep identity children))))

And then:

[<>
 "Some text."
 [:h2 "A heading"]
 "More text."
 [:h2 "Another heading"]
 "Even more text."]

work, to mirror the jsx syntax?

render() {
  return (
    <>
      Some text.
      <h2>A heading</h2>
      More text.
      <h2>Another heading</h2>
      Even more text.
    </>
  );
}

@Deraen
Copy link
Member

Deraen commented Nov 29, 2017

Interesting. I never saw that syntax but yes, that would be possible.

@nbardy
Copy link
Contributor Author

nbardy commented Dec 5, 2017

Interesting, but I prefer the fragments version. Looks better in JSX because they use <> </> and as a pair, but there isn't a clear visual connection between [<> ] with the cljs syntax.

@smw
Copy link

smw commented Dec 17, 2017

I get this when I try to return an array using the suggested snippet above:

react-dom.inc.js:17985 Uncaught Error: freshink.views.common.fragment.render(): A valid React element (or null) must be returned. You may have returned undefined, an array or some other invalid object.
    at invariant (react-dom.inc.js:17985)

@rauhs
Copy link

rauhs commented Dec 22, 2017

I'm not a reagent user but I think the CLJS community should come together and decide on a hiccup syntax to define fragments so they're the same in Sablono, Rum & Reagent (and possibly even Hiccup itself). Other tickets:

I'd personally vote for one of the following:

[:* {:key x} child0 child1]
[:* child0 child1]
[:<> child0 child1]
[:<> {:key x} child0 child1]

With preference to :* since it mimics * as "zero or more" (regex). I don't think we should use [<> ...] since <> would have to be defined and could complicate things, especially when considering server side renders (in CLJ).

As an aside: There is probably more features coming to React which might warrant more hiccup syntax:

@DjebbZ
Copy link

DjebbZ commented Dec 22, 2017

Can I chime in and "vote" ? I agree with the usage of a keyword, it makes everything much easier. My preference would go to :<> since it's the same as <> in JSX. But :* is fine too and the logic behind it is... logic (!).

@nbardy
Copy link
Contributor Author

nbardy commented Mar 16, 2018

React 16.2 Added more fragment support.

https://reactjs.org/blog/2017/11/28/react-v16.2.0-fragment-support.html

Perhaps the syntax should be [:fragment] to line up with this.

@Deraen
Copy link
Member

Deraen commented Mar 17, 2018

@nbardy Check #352 for :<> support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants