diff --git a/src/Hyperlink.tsx b/src/Hyperlink.tsx index 9a951d8..a5d0b3f 100644 --- a/src/Hyperlink.tsx +++ b/src/Hyperlink.tsx @@ -11,6 +11,17 @@ const linkify = require('linkify-it')(); const { OS } = Platform; +/** + * Copies `props` without React's special `ref` prop. `key` (and `ref` on React + * 18) is a non-enumerable warning getter that a spread skips but destructuring + * would read, and element props are frozen, so they cannot be deleted in place. + */ +const propsWithoutRef = (props: T): T => { + const rest: T & { ref?: unknown } = { ...props }; + delete rest.ref; + return rest; +}; + class Hyperlink extends Component { public static defaultProps: Partial = { linkify, @@ -32,9 +43,7 @@ class Hyperlink extends Component { } render() { - // Avoid spreading React special props such as `key` or `ref` - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { key: _key, ref: _ref, ...viewProps } = this.props as any; + const viewProps: any = propsWithoutRef(this.props); // If no link handlers or styles, just render children as-is if ( @@ -96,8 +105,7 @@ class Hyperlink extends Component { let elements: Array = []; let _lastIndex = 0; - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { ref: _ref, key: _key, ...componentProps } = component.props || {}; + const componentProps = propsWithoutRef(component.props || {}); try { this.state.linkifyIt @@ -159,8 +167,7 @@ class Hyperlink extends Component { let { props: { children } = { children: undefined } } = component || {}; if (!children) return component; - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { ref: _ref, key: _key, ...componentProps } = component.props || {}; + const componentProps = propsWithoutRef(component.props || {}); return React.cloneElement( component, @@ -213,10 +220,7 @@ export default class extends Component { render() { const onPress = this.props.onPress ?? this.handleLink; - // Do not forward `key`/`ref` to the inner `Hyperlink` to avoid - // React warning about spreading a props object that contains `key`. - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { key: _key, ref: _ref, ...rest } = this.props as any; + const rest: any = propsWithoutRef(this.props); return this.props.linkDefault ? ( { + let errorSpy: jest.SpyInstance; + + beforeEach(() => { + errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + errorSpy.mockRestore(); + }); + + const keyWarnings = () => + errorSpy.mock.calls.filter(call => + String(call[0]).includes('`key` is not a prop'), + ); + + // React logs that warning only once per process, so each case builds its + // keyed element with a freshly required React to stay independent. + const createKeyedElement = ( + type: ElementType, + props: Record, + ...children: ReactNode[] + ): ReactElementWithType => { + let element: ReactElementWithType | undefined; + jest.isolateModules(() => { + const { createElement } = require('react'); + element = createElement(type, props, ...children); + }); + return element as ReactElementWithType; + }; + + it('should not read `key` when linkifying a keyed Text', () => { + render( + + {createKeyedElement(Text, { key: 'a' }, 'see https://example.com')} + , + ); + + expect(keyWarnings()).toHaveLength(0); + }); + + it('should not read `key` when parsing a keyed non-Text child', () => { + render( + + {createKeyedElement( + View, + { key: 'b' }, + see https://example.com, + )} + , + ); + + expect(keyWarnings()).toHaveLength(0); + }); + + it('should not read `key` when the Hyperlink itself has a key', () => { + render( + createKeyedElement( + Hyperlink, + { key: 'c', linkStyle: { color: 'blue' } }, + see https://example.com, + ), + ); + + expect(keyWarnings()).toHaveLength(0); + }); + + it('should not mutate the frozen props of the elements it processes', () => { + const child = createKeyedElement( + Text, + { key: 'd', testID: 'child' }, + 'see https://example.com', + ); + expect(Object.isFrozen(child.props)).toBe(true); + + expect(() => + render({child}), + ).not.toThrow(); + + expect(child.props.testID).toBe('child'); + }); +});