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.

Sign up for GitHub

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

Jump to bottom

Remove children from props of ErrorMessage when using component #87

Merged
merged 2 commits into from
Dec 3, 2021
Merged

Remove children from props of ErrorMessage when using component #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1185eb2
Add tests for `as` component (failing typescript)
IanVS Sep 22, 2021
f0d5c51
Remove `children` from props of ErrorMessage when using component
IanVS Sep 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 37 additions & 6 deletions src/ErrorMessage.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('ErrorMessage', () => {
expect(asFragment()).toMatchSnapshot();
});

it('should render correctly with flat errors and as with component and render', () => {
it('should render correctly with flat errors and as with element and render', () => {
const { asFragment } = render(
<ErrorMessage
as={<span />}
Expand All @@ -58,7 +58,7 @@ describe('ErrorMessage', () => {
expect(asFragment()).toMatchSnapshot();
});

it('should render correctly with flat errors and as with component and className and render', () => {
it('should render correctly with flat errors and as with element and className and render', () => {
const { asFragment } = render(
<ErrorMessage
as={<span />}
Expand All @@ -72,6 +72,37 @@ describe('ErrorMessage', () => {
expect(asFragment()).toMatchSnapshot();
});

it('should render correctly with flat errors and as with component', () => {
function CustErrComp({ children }: { children: React.ReactNode }) {
return <div>{children}</div>;
}
const { asFragment } = render(
<ErrorMessage
as={CustErrComp}
errors={{ flat: { type: 'flat', message: 'flat' } }}
name="flat"
/>,
);

expect(asFragment()).toMatchSnapshot();
});

it('should render correctly with flat errors and as with component and render', () => {
function CustErrComp({ children }: { children: React.ReactNode }) {
return <div>{children}</div>;
}
const { asFragment } = render(
<ErrorMessage
as={CustErrComp}
errors={{ flat: { type: 'flat', message: 'flat' } }}
name="flat"
render={({ message }) => message}
/>,
);

expect(asFragment()).toMatchSnapshot();
});

it('should render correctly with flat multiple errors and render', () => {
const { asFragment } = render(
<ErrorMessage
Expand Down Expand Up @@ -99,7 +130,7 @@ describe('ErrorMessage', () => {
expect(asFragment()).toMatchSnapshot();
});

it('should render correctly with flat multiple errors and as with component and render', () => {
it('should render correctly with flat multiple errors and as with element and render', () => {
const { asFragment } = render(
<ErrorMessage
as={<div />}
Expand Down Expand Up @@ -204,7 +235,7 @@ describe('ErrorMessage', () => {
expect(asFragment()).toMatchSnapshot();
});

it('should render correctly with nested multiple errors and as with component and render', () => {
it('should render correctly with nested multiple errors and as with element and render', () => {
const { asFragment } = render(
<ErrorMessage
as={<div />}
Expand Down Expand Up @@ -269,7 +300,7 @@ describe('ErrorMessage', () => {
expect(asFragment()).toMatchSnapshot();
});

it('should render correctly with nested errors array and as with component and render', () => {
it('should render correctly with nested errors array and as with element and render', () => {
const { asFragment } = render(
<ErrorMessage
as={<span />}
Expand Down Expand Up @@ -319,7 +350,7 @@ describe('ErrorMessage', () => {
expect(asFragment()).toMatchSnapshot();
});

it('should render correctly with nested multiple errors array and as with component and render', () => {
it('should render correctly with nested multiple errors array and as with element and render', () => {
const { asFragment } = render(
<ErrorMessage
as={<div />}
Expand Down
26 changes: 20 additions & 6 deletions src/__snapshots__/ErrorMessage.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,21 @@ exports[`ErrorMessage should render correctly with flat errors 1`] = `
</DocumentFragment>
`;

exports[`ErrorMessage should render correctly with flat errors and as with component and className and render 1`] = `
exports[`ErrorMessage should render correctly with flat errors and as with component 1`] = `
<DocumentFragment>
<div>
flat
</div>
</DocumentFragment>
`;

exports[`ErrorMessage should render correctly with flat errors and as with component and render 1`] = `
<DocumentFragment>
flat
</DocumentFragment>
Comment on lines +19 to +22
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks incorrect. According to the README, the as should take precedence, but that seems to only apply to elements provided in as, not components.

Copy link
Member

Choose a reason for hiding this comment

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

@lanvs oh... you are right!
We need to fix this.

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 need me to try to work on it in this PR, or can that maybe be a separate effort?

Copy link
Member

Choose a reason for hiding this comment

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

Let's merge this once 👍🏻
If you are willing to help, we would like you to create another PR 🙏🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll see what I can do, but TBH I'm stretched a bit thin at the moment.

`;

exports[`ErrorMessage should render correctly with flat errors and as with element and className and render 1`] = `
<DocumentFragment>
<span
class="test"
Expand All @@ -18,7 +32,7 @@ exports[`ErrorMessage should render correctly with flat errors and as with compo
</DocumentFragment>
`;

exports[`ErrorMessage should render correctly with flat errors and as with component and render 1`] = `
exports[`ErrorMessage should render correctly with flat errors and as with element and render 1`] = `
<DocumentFragment>
<span>
flat
Expand All @@ -44,7 +58,7 @@ exports[`ErrorMessage should render correctly with flat errors and as with strin
</DocumentFragment>
`;

exports[`ErrorMessage should render correctly with flat multiple errors and as with component and render 1`] = `
exports[`ErrorMessage should render correctly with flat multiple errors and as with element and render 1`] = `
<DocumentFragment>
<div>
flat
Expand Down Expand Up @@ -86,7 +100,7 @@ exports[`ErrorMessage should render correctly with nested errors array 1`] = `
</DocumentFragment>
`;

exports[`ErrorMessage should render correctly with nested errors array and as with component and render 1`] = `
exports[`ErrorMessage should render correctly with nested errors array and as with element and render 1`] = `
<DocumentFragment>
<span>
array
Expand Down Expand Up @@ -116,7 +130,7 @@ exports[`ErrorMessage should render correctly with nested errors object and as w
</DocumentFragment>
`;

exports[`ErrorMessage should render correctly with nested multiple errors and as with component and render 1`] = `
exports[`ErrorMessage should render correctly with nested multiple errors and as with element and render 1`] = `
<DocumentFragment>
<div>
object
Expand All @@ -138,7 +152,7 @@ exports[`ErrorMessage should render correctly with nested multiple errors and re
</DocumentFragment>
`;

exports[`ErrorMessage should render correctly with nested multiple errors array and as with component and render 1`] = `
exports[`ErrorMessage should render correctly with nested multiple errors array and as with element and render 1`] = `
<DocumentFragment>
<div>
array
Expand Down
2 changes: 1 addition & 1 deletion src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type AsProps<TAs> = TAs extends undefined
: TAs extends React.ReactElement
? Record<string, any>
: TAs extends React.ComponentType<infer P>
? P
? Omit<P, 'children'>
: TAs extends keyof JSX.IntrinsicElements
? JSX.IntrinsicElements[TAs]
: never;
Expand Down