-
-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
React fiber support #1443
React fiber support #1443
Changes from 1 commit
e780cc5
ef61f91
0722584
5b14eae
67f496d
90c6253
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`element_check.utils.flattenList should flatten a deep nested list 1`] = ` | ||
Array [ | ||
1, | ||
2, | ||
3, | ||
4, | ||
5, | ||
6, | ||
7, | ||
] | ||
`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import React from 'react'; | ||
|
||
// return true if the element is renderable with react fiber | ||
export const isValidFiberElement = element => | ||
typeof element === 'string' || typeof element === 'number' || React.isValidElement(element); | ||
|
||
// input: [1, 2, [3, 4, [5, 6, [7]]]] | ||
// output: [ 1, 2, 3, 4, 5, 6, 7 ] | ||
export const flattenList = list => | ||
list.reduce((array, element) => { | ||
const newElement = Array.isArray(element) ? flattenList(element) : [element]; | ||
|
||
return [...array, ...newElement]; | ||
}, []); | ||
|
||
export const isPriorToFiber = version => { | ||
const [majorVersion] = version.split('.'); | ||
|
||
return Number(majorVersion) < 16; | ||
}; | ||
|
||
// accepts an element and return true if renderable else return false | ||
const isReactRenderable = element => { | ||
// storybook is running with a version prior to fiber, | ||
// run a simple check on the element | ||
if (isPriorToFiber(React.version)) { | ||
return React.isValidElement(element); | ||
} | ||
|
||
// the element is not an array, check if its a fiber renderable element | ||
if (!Array.isArray(element)) { | ||
return isValidFiberElement(element); | ||
} | ||
|
||
// the element is in fact a list of elements (array), | ||
// loop on its elements to see if its ok to render them | ||
const elementsList = element.map(isReactRenderable); | ||
|
||
// flatten the list of elements (possibly deep nested) | ||
const flatList = flattenList(elementsList); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is it possibly deep nested if you are running There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hehe, I personally wouldn't design a component deep nested, but it could be possible: const WeirdComponent = () => [
<div>Some list</div>,
[
<Something />,
<div>This component is technically valid</div>,
<em>But weird</em>
],
<strong>Y U LIST</strong>,
]; Note: in this case, we should maybe throw an error: "Break this component in smaller ones for your sanity sake!" ๐ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But if element is an array, then array.map(fn) is going to return an array, not a deeply-nested array? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh sorry, it's recursive. nevermind ๐ |
||
|
||
// keep only invalid elements | ||
const invalidElements = flatList.filter(elementIsRenderable => elementIsRenderable === false); | ||
|
||
// it's ok to render this list if there is no invalid elements inside | ||
return !invalidElements.length; | ||
}; | ||
|
||
export default isReactRenderable; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
import React from 'react'; | ||
import isReactRenderable, { | ||
isValidFiberElement, | ||
flattenList, | ||
isPriorToFiber, | ||
} from './element_check'; | ||
|
||
describe('element_check.utils.isValidFiberElement', () => { | ||
it('should accept to render a string', () => { | ||
const string = 'react is awesome'; | ||
|
||
expect(isValidFiberElement(string)).toBe(true); | ||
}); | ||
|
||
it('should accept to render a number', () => { | ||
const number = 42; | ||
|
||
expect(isValidFiberElement(number)).toBe(true); | ||
}); | ||
|
||
it('should accept to render a valid React element', () => { | ||
const element = <button>Click me</button>; | ||
|
||
expect(isValidFiberElement(element)).toBe(true); | ||
}); | ||
|
||
it("shouldn't accept to render an arbitrary object", () => { | ||
const object = { key: 'bee bop' }; | ||
|
||
expect(isValidFiberElement(object)).toBe(false); | ||
}); | ||
|
||
it("shouldn't accept to render a function", () => { | ||
const noop = () => {}; | ||
|
||
expect(isValidFiberElement(noop)).toBe(false); | ||
}); | ||
|
||
it("shouldn't accept to render undefined", () => { | ||
expect(isValidFiberElement(undefined)).toBe(false); | ||
}); | ||
}); | ||
|
||
describe('element_check.utils.flattenList', () => { | ||
it('should flatten a deep nested list', () => { | ||
const deepNestedList = [1, 2, [3, 4, [5, 6, [7]]]]; | ||
|
||
expect(flattenList(deepNestedList)).toMatchSnapshot(); | ||
}); | ||
}); | ||
|
||
describe('element_check.utils.isPriorToFiber', () => { | ||
it('should return true if React version is prior to Fiber (< 16)', () => { | ||
const oldVersion = '0.14.5'; | ||
const version = '15.5.4'; | ||
|
||
expect(isPriorToFiber(oldVersion)).toBe(true); | ||
expect(isPriorToFiber(version)).toBe(true); | ||
}); | ||
|
||
it('should return false if React version is using Fiber features (>= 16)', () => { | ||
const alphaVersion = '16.0.0-alpha.13'; | ||
const version = '18.3.1'; | ||
|
||
expect(isPriorToFiber(alphaVersion)).toBe(false); | ||
expect(isPriorToFiber(version)).toBe(false); | ||
}); | ||
}); | ||
|
||
describe('element_check.isReactRenderable', () => { | ||
const string = 'yo'; | ||
const number = 1337; | ||
const element = <span>what's up</span>; | ||
const array = [string, number, element]; | ||
const object = { key: null }; | ||
|
||
it('allows rendering React elements only prior to React Fiber', () => { | ||
// mutate version for the purpose of the test | ||
React.version = '15.5.4'; | ||
|
||
expect(isReactRenderable(string)).toBe(false); | ||
expect(isReactRenderable(number)).toBe(false); | ||
expect(isReactRenderable(element)).toBe(true); | ||
expect(isReactRenderable(array)).toBe(false); | ||
expect(isReactRenderable(object)).toBe(false); | ||
}); | ||
|
||
it('allows rendering string, numbers, arrays and React elements with React Fiber', () => { | ||
// mutate version for the purpose of the test | ||
React.version = '16.0.0-alpha.13'; | ||
|
||
expect(isReactRenderable(string)).toBe(true); | ||
expect(isReactRenderable(number)).toBe(true); | ||
expect(isReactRenderable(element)).toBe(true); | ||
expect(isReactRenderable(array)).toBe(true); | ||
expect(isReactRenderable(object)).toBe(false); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import { flattenDeep } from 'lodash'
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
๐ definitely! I was hesitant to import
lodash
as I saw very few use of it in the project! ๐ will replace this function bylodash.flattenDeep
!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated! โ