Skip to content
This repository has been archived by the owner on Aug 1, 2018. It is now read-only.

Support React.Fragment #84

Merged
merged 3 commits into from
Mar 26, 2018

Conversation

KevinGrandon
Copy link
Contributor

@KevinGrandon KevinGrandon commented Mar 22, 2018

  • Check for React.Fragment case in prepareElement.
  • Adds tests for supporting React.Fragment

Fixes #84

@KevinGrandon KevinGrandon changed the title WIP - React.Fragment Support React.Fragment Mar 22, 2018
@codecov
Copy link

codecov bot commented Mar 22, 2018

Codecov Report

Merging #84 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #84   +/-   ##
=======================================
  Coverage   75.37%   75.37%           
=======================================
  Files           8        8           
  Lines         134      134           
  Branches       31       31           
=======================================
  Hits          101      101           
  Misses         21       21           
  Partials       12       12
Impacted Files Coverage Δ
src/prepare.js 89.74% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fda3fb2...980f284. Read the comment docs.

src/prepare.js Outdated
if (typeof type === 'string') {
if (
typeof type === 'string' ||
type.toString() === React.Fragment.toString()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not type === React.Fragment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is better, updated. Thanks!

ganemone
ganemone previously approved these changes Mar 22, 2018
@@ -41,7 +41,7 @@ function prepareElement(element, context) {
return Promise.resolve([null, context]);
}
const {type, props} = element;
if (typeof type === 'string') {
if (typeof type === 'string' || type === React.Fragment) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we should still traverse the children here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a code example where this would be leveraged? I checked with a simple <div> element, and this was the path that we fall into, so I imagined it would be similar to a div case. Seems like we should handle children the same in both cases?

dennisgl
dennisgl previously approved these changes Mar 23, 2018
@KevinGrandon KevinGrandon dismissed stale reviews from dennisgl and ganemone via 980f284 March 24, 2018 07:49
@KevinGrandon
Copy link
Contributor Author

!merge

@old-fusion-bot old-fusion-bot bot merged commit 96a8d29 into fusionjs:master Mar 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants