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

Update blacklisted props that can be passed to native HTML elements using the transferProps principle #520

Merged
merged 1 commit into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions src/components/_helpers/__tests__/transferProps.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
const props = {
propA: 'value',
propB: 'value',
propC: 'value',
};
const expectedProps = { ...props };

Expand All @@ -14,13 +13,20 @@

it('returns filtered props using always blacklisted props', () => {
const props = {
children: 'value',
className: 'value',
ref: 'value',
staticContext: 'value',
contentEditable: true,
propA: 'value',
};
const expectedProps = {};
const expectedProps = { propA: 'value' };

let errorString;
const originalConsoleError = console.error;

Check warning on line 23 in src/components/_helpers/__tests__/transferProps.test.js

View workflow job for this annotation

GitHub Actions / Lint

Unexpected console statement
console.error = (error) => {

Check warning on line 24 in src/components/_helpers/__tests__/transferProps.test.js

View workflow job for this annotation

GitHub Actions / Lint

Unexpected console statement
errorString = error;
};
expect(transferProps(props)).toEqual(expectedProps);
expect(errorString).toEqual('Invalid prop(s) supplied to the "transferProps" function: "className", "contentEditable"');

console.error = originalConsoleError;

Check warning on line 30 in src/components/_helpers/__tests__/transferProps.test.js

View workflow job for this annotation

GitHub Actions / Lint

Unexpected console statement
});
});
54 changes: 47 additions & 7 deletions src/components/_helpers/transferProps.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,47 @@
export const transferProps = ({
children,
className,
ref,
staticContext,
...restProps
}) => restProps;
/**
* Controls passing of props from the React component to the HTML element
*
* Sometimes it is useful to have a mechanism to pass props from the React component to a rendered HTML element.
* It enables making the component interactive and helps improve its accessibility. However some props should
* never be passed to the HTML element as it would break things. This function is used to filter out such props.
*
* When run in development mode, the function will log the error to the console if any invalid props are passed.
*
* @param props The props that were passed to the React component and were not used by it
* @returns The props to be passed to the HTML element
*/
export const transferProps = (props) => {
const {
children,
className,
contentEditable,
dangerouslySetInnerHTML,
ref,
staticContext,
style,
suppressContentEditableWarning,
...restProps
} = props;

if (process.env.NODE_ENV !== 'production') {
mbohal marked this conversation as resolved.
Show resolved Hide resolved
console.log('props', props);

Check warning on line 27 in src/components/_helpers/transferProps.js

View workflow job for this annotation

GitHub Actions / Lint

Unexpected console statement
const invalidProps = [
'children', // It is always either handled by the component itself or not supported.
'className', // Classes are set by component authors, changing it arbitrarily might break things.
'contentEditable', // Components are either interactive or not, changing it arbitrarily might break things.
'dangerouslySetInnerHTML', // This might break things and allow for XSS attacks.
'ref', // Forwarding `ref` is hardcoded and documented for each component.
'staticContext', // In `react-router` (v4, v5) this is used during server side rendering, it makes no sense to pass it to a component.
'style', // Styles are set by component authors, changing it arbitrarily might break things.
'suppressContentEditableWarning', // Since setting `contentEditable` is not allowed, this is not needed.
]
.filter((key) => props[key] !== undefined);

if (invalidProps.length > 0) {
// eslint-disable-next-line no-console
console.error(`Invalid prop(s) supplied to the "transferProps" function: "${invalidProps.join('", "')}"`);
}
}

return restProps;
};
Loading