Skip to content

Commit

Permalink
updated behavior of the children prop to be handled by the merge func…
Browse files Browse the repository at this point in the history
…tion
  • Loading branch information
patrickjames242 committed Mar 19, 2023
1 parent d06bf12 commit a7b233c
Show file tree
Hide file tree
Showing 8 changed files with 228 additions and 40 deletions.
45 changes: 44 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,47 @@ export const MyButton = ComponentBuilders.button(Button => {
// the props passed to the underlying button element are as follows: { onClick: "foo" }
```
- **The `children` prop:** The inner children prop will always override the outer prop if the inner prop is anything other than undefined.
```tsx
import { ComponentBuilders } from './ComponentBuilders';

export const MyButton = ComponentBuilders.button<{
children?: string
}>(Button => {
return <Button>
My Button {/* If specified, this value will override any children prop provided in the outer props */}
</Button>
});

<MyButton>
My Customized Title {/* This value will be ignored because the inner children prop has been specified */}
</MyButton>

// the resulting children value will be: "My Button"
```
If you would like to allow users to customize the children of the component, pluck the `children` prop.
```tsx
import { ComponentBuilders } from './ComponentBuilders';

export const MyButton = ComponentBuilders.button<{
children?: string
}>((Button, props) => {
const { children } = props.pluck('children');
return <Button>
{ children ?? 'My Button'}
</Button>
});

<MyButton>
My Customized Title
</MyButton>

// the resulting children value will be: "My Customized Title"
```
- **Any Other Prop:** For any other prop, the outer props will override the inner props.
```tsx
import { ComponentBuilders } from './ComponentBuilders';
Expand Down Expand Up @@ -544,9 +585,11 @@ You are provided with the `innerProps`, which are the props that were passed to
Because refs are treated like regular props in this library, the `ref` passed to the outermost component will be included in `outerProps` and the `ref` passed to the inner component will be included in `innerProps`. This means you would be responsible for ensuring these are merged properly. You may use the `mergeRefs` function exported by this library if you so desire.
Similarly, you will also be responsible for merging the `children` props together.
Note that you will only receive props in the `outerProps` object that were not 'plucked' within the component.
You are also provided with the `defaultMergeFn` in the merge function, which you may find useful if you'd just like to merge a specific prop but have the library handle the rest.
You are provided with the `defaultMergeFn` in the merge function, which you may find useful if you'd just like to merge a specific prop but have the library handle the rest.
```tsx
import { createComponentBuilderGroup } from 'react-extend-components';
Expand Down
5 changes: 1 addition & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@
"format": "prettier --write ./**/*.{js,jsx,ts,tsx,html,css,scss,json}",
"lint": "eslint ./src --ext .js,.jsx,.ts,.tsx --max-warnings=0",
"lint:fix": "eslint ./src --ext .js,.jsx,.ts,.tsx --fix",
"prepublishOnly": "npm run lint && npm run build",
"preversion": "npm run lint",
"version": "npm run format && git add -A src",
"postversion": "git push && git push --tags",
"prepublishOnly": "npm run lint && npm run test && npm run build",
"test": "jest",
"test:watch": "jest --watch"
},
Expand Down
136 changes: 127 additions & 9 deletions src/createComponentBuilderGetter.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { create } from 'react-test-renderer';
import { createComponentBuilderGetter } from './createComponentBuilderGetter';
import { createWithAct } from './testUtils/createWithAct';

test('passes props to underlying element', () => {
const TestComponent = createComponentBuilderGetter('div')((Div) => {
Expand All @@ -21,18 +20,32 @@ test('passes props to underlying element', () => {
});

test('plucked props are not passed to the underlying element', () => {
let propsToPluck: string[];
const TestComponent = createComponentBuilderGetter('section')(
(Section, props) => {
props.pluck('style', 'className');
props.pluck(...(propsToPluck as any));
return <Section />;
}
);

const propsPassedToElement = create(
propsToPluck = ['style', 'className'];
const component = create(
<TestComponent style={{ color: 'red' }} className="patrick" id="blah" />
).root.findByType('section').props;
);
const getProps = (): any => component.root.findByType('section').props;
expect(getProps()).toEqual({ id: 'blah' });

expect(propsPassedToElement).toEqual({ id: 'blah' });
propsToPluck = ['tabIndex', 'hidden'];
component.update(
<TestComponent
style={{ color: 'red' }}
className="patrick"
tabIndex={-1}
hidden
/>
);

expect(getProps()).toEqual({ style: { color: 'red' }, className: 'patrick' });
});

test('plucked props are returned from the pluck function', () => {
Expand All @@ -48,7 +61,7 @@ test('plucked props are returned from the pluck function', () => {

propsToPluck = ['style', 'className'];

const created = createWithAct(
const component = create(
<TestComponent style={{ color: 'red' }} className="patrick" id="blah" />
);

Expand All @@ -59,7 +72,7 @@ test('plucked props are returned from the pluck function', () => {

propsToPluck = ['id', 'style'];

created.update(
component.update(
<TestComponent
style={{ backgroundColor: 'green' }}
className="patrick123"
Expand All @@ -79,14 +92,119 @@ test('when using pluckAll, no props are passed to the underlying element', () =>
return <P />;
});

const created = createWithAct(
const component = create(
<TestComponent
style={{ color: 'red' }}
className="some-class-name"
id="blah"
/>
);

const getProps = (): any => created.root.findByType('p').props;
const getProps = (): any => component.root.findByType('p').props;

component.update(
<TestComponent
tabIndex={35}
className="some-class-name"
id="blah"
onClick={() => {}}
/>
);

expect(getProps()).toEqual({});
});

test('refs are passed to the underlying element by default', () => {
// const TestComponent = createComponentBuilderGetter('div')((Div) => {
// return <Div />;
// });
// const ref1 = jest.fn();
// const component = create(<TestComponent ref={ref1} />);
// const getProps = (): any => component.root.findByType('div').props;
// expect(getProps()).toEqual({ ref: expect.any(Function) });
});

test('pluck function returns refs and hides them from the underlying element', () => {
// const receivePluckedProps = jest.fn();
// const TestComponent = createComponentBuilderGetter('div')((Div, props) => {
// receivePluckedProps(props.pluck('ref'));
// return <Div />;
// });
// const component = create(<TestComponent />);
// const getProps = (): any => component.root.findByType('div').props;
// expect(getProps()).toEqual({ ref: expect.any(Function) });
});

test('Peek returns all props, including those that are plucked', () => {
let propsToPluck: string[] = [];
const receivePeekedProps = jest.fn();

const TestComponent = createComponentBuilderGetter('input')(
(Input, props) => {
props.pluck(...(propsToPluck as any));
receivePeekedProps(props.peek());
return <Input />;
}
);

propsToPluck = ['style', 'className'];

const component = create(
<TestComponent style={{ color: 'red' }} className="patrick" id="blah" />
);

expect(receivePeekedProps).toHaveBeenNthCalledWith(1, {
style: { color: 'red' },
className: 'patrick',
id: 'blah',
});

propsToPluck = ['id', 'style'];

component.update(
<TestComponent
style={{ backgroundColor: 'green' }}
className="patrick123"
id="7"
/>
);

expect(receivePeekedProps).toHaveBeenNthCalledWith(2, {
style: { backgroundColor: 'green' },
className: 'patrick123',
id: '7',
});
});

test('peeked props are always passed to the underlying element', () => {
const TestComponent = createComponentBuilderGetter('div')((Div, props) => {
props.peek();
return <Div />;
});

const component = create(
<TestComponent style={{ color: 'red' }} className="patrick" id="blah" />
);

const getProps = (): any => component.root.findByType('div').props;

expect(getProps()).toEqual({
style: { color: 'red' },
className: 'patrick',
id: 'blah',
});

component.update(
<TestComponent
style={{ backgroundColor: 'green' }}
className="patrick123"
id="7"
/>
);

expect(getProps()).toEqual({
style: { backgroundColor: 'green' },
className: 'patrick123',
id: '7',
});
});
4 changes: 2 additions & 2 deletions src/createComponentBuilderGetter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export const createComponentBuilderGetter: ComponentBuilderGetter = <
const Fn: ForwardRefRenderFunction<
ReactTagProps<RootTag>['ref'],
ReactTagProps<RootTag>
> = ({ children, ...innerProps }, innerRef) => {
> = (innerProps, innerRef) => {
const { outerProps, pluckedProps, pluckAllProps } =
useConsumeObservableValue(observableValues);

Expand Down Expand Up @@ -99,7 +99,7 @@ export const createComponentBuilderGetter: ComponentBuilderGetter = <
defaultMergeFn: defaultPropsMergeFn,
});

return createElement(rootTag, mergedProps, children);
return createElement(rootTag, mergedProps);
};
return forwardRef(Fn) as any;
},
Expand Down
48 changes: 48 additions & 0 deletions src/defaultPropsMergeFn.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,54 @@ describe('function prop merging', () => {
});
});

describe('children prop merging', () => {
test('inner value overrides outer value if inner value is set', () => {
const resultProps = defaultPropsMergeFn({
outerProps: {
children: 'outer-value',
},
innerProps: {
children: 'inner-value',
},
});
expect(resultProps.children).toEqual('inner-value');

const resultProps2 = defaultPropsMergeFn({
outerProps: {
children: 'outer-value',
},
innerProps: {
children: null,
},
});
expect(resultProps2.children).toEqual(null);
});

describe("outer value is used when inner value isn't set", () => {
test('when inner value is undefined', () => {
const resultProps = defaultPropsMergeFn({
outerProps: {
children: 'outer-value',
},
innerProps: {
children: undefined,
},
});
expect(resultProps.children).toEqual('outer-value');
});

test('when inner value is not included', () => {
const resultProps = defaultPropsMergeFn({
outerProps: {
children: 'outer-value',
},
innerProps: {},
});
expect(resultProps.children).toEqual('outer-value');
});
});
});

describe('general prop merging', () => {
test('outer props override inner props where needed', () => {
const resultProps = defaultPropsMergeFn({
Expand Down
4 changes: 3 additions & 1 deletion src/defaultPropsMergeFn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ export const defaultPropsMergeFn: DefaultPropsMergeFn = ({
outerProps
);

//TODO: reverse this so outer props override inner props
if ('children' in innerProps && innerProps.children !== undefined) {
mergedProps.children = innerProps.children;
}

for (const key in outerProps) {
if (!(key in innerProps)) continue;
Expand Down
20 changes: 0 additions & 20 deletions src/testUtils/createWithAct.tsx

This file was deleted.

6 changes: 3 additions & 3 deletions src/utils/mergeRefs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ test('setting the merged ref sets all the refs', () => {

const mergedRefs = mergeRefs(mockRefCallback, refObj1, refObj2);
setRefValue(mergedRefs!, 'foo');
expect(mockRefCallback).toHaveBeenCalledWith('foo');
expect(mockRefCallback).toHaveBeenNthCalledWith(1, 'foo');
expect(refObj1.current).toBe('foo');
expect(refObj2.current).toBe('foo');

setRefValue(mergedRefs!, null);
expect(mockRefCallback).toHaveBeenCalledWith(null);
expect(mockRefCallback).toHaveBeenNthCalledWith(2, null);
expect(refObj1.current).toBe(null);
expect(refObj2.current).toBe(null);

setRefValue(mergedRefs!, 'bar');
expect(mockRefCallback).toHaveBeenCalledWith('bar');
expect(mockRefCallback).toHaveBeenNthCalledWith(3, 'bar');
expect(refObj1.current).toBe('bar');
expect(refObj2.current).toBe('bar');
});
Expand Down

0 comments on commit a7b233c

Please sign in to comment.