From 6d7a1eaf0e1c773ccea11537be5e79b306605e35 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Fri, 23 Apr 2021 11:00:14 -0700 Subject: [PATCH] [change] Remove createElement host type inference for 'href' Elements other than 'a' support the 'href' attribute, so the element type should not be inferred from 'href'. Longer term no host element type inference should be performed by 'createElement'. Fix #1958 --- .../__snapshots__/index-test.js.snap | 14 +++++++--- .../exports/Pressable/__tests__/index-test.js | 7 ++++- .../__snapshots__/index-test.js.snap | 6 ++--- .../src/exports/Text/__tests__/index-test.js | 2 +- .../src/exports/Text/index.js | 25 ++++++++++-------- .../__snapshots__/index-test.js.snap | 6 ++--- .../src/exports/View/__tests__/index-test.js | 2 +- .../src/exports/View/index.js | 26 +++++++++++-------- .../__snapshots__/index-test.js.snap | 8 ++++++ .../createElement/__tests__/index-test.js | 6 ++++- .../propsToAccessibilityComponent.js | 6 ----- 11 files changed, 67 insertions(+), 41 deletions(-) diff --git a/packages/react-native-web/src/exports/Pressable/__tests__/__snapshots__/index-test.js.snap b/packages/react-native-web/src/exports/Pressable/__tests__/__snapshots__/index-test.js.snap index 3c1abf40..8d2f4f20 100644 --- a/packages/react-native-web/src/exports/Pressable/__tests__/__snapshots__/index-test.js.snap +++ b/packages/react-native-web/src/exports/Pressable/__tests__/__snapshots__/index-test.js.snap @@ -106,9 +106,10 @@ exports[`components/Pressable prop "accessibilityLiveRegion" value is set 1`] = `; exports[`components/Pressable prop "accessibilityRole" value alters HTML element 1`] = ` - `; @@ -136,6 +137,13 @@ exports[`components/Pressable prop "disabled" 1`] = ` /> `; +exports[`components/Pressable prop "href" 1`] = ` + +`; + exports[`components/Pressable prop "nativeID" value is set 1`] = `
{ }); test('value alters HTML element', () => { - const { container } = render(); + const { container } = render(); expect(container.firstChild).toMatchSnapshot(); }); }); @@ -48,6 +48,11 @@ describe('components/Pressable', () => { expect(container.firstChild).toMatchSnapshot(); }); + test('prop "href"', () => { + const { container } = render(); + expect(container.firstChild).toMatchSnapshot(); + }); + describe('prop "nativeID"', () => { test('value is set', () => { const { container } = render(); diff --git a/packages/react-native-web/src/exports/Text/__tests__/__snapshots__/index-test.js.snap b/packages/react-native-web/src/exports/Text/__tests__/__snapshots__/index-test.js.snap index 0b8e3bd0..37b7c11c 100644 --- a/packages/react-native-web/src/exports/Text/__tests__/__snapshots__/index-test.js.snap +++ b/packages/react-native-web/src/exports/Text/__tests__/__snapshots__/index-test.js.snap @@ -51,10 +51,10 @@ exports[`components/Text prop "accessibilityLiveRegion" value is set 1`] = ` `; exports[`components/Text prop "accessibilityRole" value alters HTML element 1`] = ` - `; diff --git a/packages/react-native-web/src/exports/Text/__tests__/index-test.js b/packages/react-native-web/src/exports/Text/__tests__/index-test.js index 01fec90c..0359dc2c 100644 --- a/packages/react-native-web/src/exports/Text/__tests__/index-test.js +++ b/packages/react-native-web/src/exports/Text/__tests__/index-test.js @@ -51,7 +51,7 @@ describe('components/Text', () => { }); test('value alters HTML element', () => { - const { container } = render(); + const { container } = render(); expect(container.firstChild).toMatchSnapshot(); }); }); diff --git a/packages/react-native-web/src/exports/Text/index.js b/packages/react-native-web/src/exports/Text/index.js index a40cf163..36f8bc0b 100644 --- a/packages/react-native-web/src/exports/Text/index.js +++ b/packages/react-native-web/src/exports/Text/index.js @@ -114,7 +114,7 @@ const Text: React.AbstractComponent = } } - const component = hasTextAncestor ? 'span' : 'div'; + let component = hasTextAncestor ? 'span' : 'div'; const supportedProps = pickProps(props); supportedProps.classList = classList; supportedProps.dir = dir; @@ -124,16 +124,19 @@ const Text: React.AbstractComponent = } supportedProps.onClick = handleClick; supportedProps.style = style; - if (props.href != null && hrefAttrs != null) { - const { download, rel, target } = hrefAttrs; - if (download != null) { - supportedProps.download = download; - } - if (rel != null) { - supportedProps.rel = rel; - } - if (typeof target === 'string') { - supportedProps.target = target.charAt(0) !== '_' ? '_' + target : target; + if (props.href != null) { + component = 'a'; + if (hrefAttrs != null) { + const { download, rel, target } = hrefAttrs; + if (download != null) { + supportedProps.download = download; + } + if (rel != null) { + supportedProps.rel = rel; + } + if (typeof target === 'string') { + supportedProps.target = target.charAt(0) !== '_' ? '_' + target : target; + } } } diff --git a/packages/react-native-web/src/exports/View/__tests__/__snapshots__/index-test.js.snap b/packages/react-native-web/src/exports/View/__tests__/__snapshots__/index-test.js.snap index 41a07887..b8cfffb5 100644 --- a/packages/react-native-web/src/exports/View/__tests__/__snapshots__/index-test.js.snap +++ b/packages/react-native-web/src/exports/View/__tests__/__snapshots__/index-test.js.snap @@ -46,9 +46,9 @@ exports[`components/View prop "accessibilityLiveRegion" value is set 1`] = ` `; exports[`components/View prop "accessibilityRole" value alters HTML element 1`] = ` - `; diff --git a/packages/react-native-web/src/exports/View/__tests__/index-test.js b/packages/react-native-web/src/exports/View/__tests__/index-test.js index 3a948155..592db3f6 100644 --- a/packages/react-native-web/src/exports/View/__tests__/index-test.js +++ b/packages/react-native-web/src/exports/View/__tests__/index-test.js @@ -79,7 +79,7 @@ describe('components/View', () => { }); test('value alters HTML element', () => { - const { container } = render(); + const { container } = render(); expect(container.firstChild).toMatchSnapshot(); }); }); diff --git a/packages/react-native-web/src/exports/View/index.js b/packages/react-native-web/src/exports/View/index.js index ae7fbddd..13c98b7b 100644 --- a/packages/react-native-web/src/exports/View/index.js +++ b/packages/react-native-web/src/exports/View/index.js @@ -97,21 +97,25 @@ const View: React.AbstractComponent = onStartShouldSetResponderCapture }); + let component = 'div'; const style = StyleSheet.compose(hasTextAncestor && styles.inline, props.style); const supportedProps = pickProps(props); supportedProps.classList = classList; supportedProps.style = style; - if (props.href != null && hrefAttrs != null) { - const { download, rel, target } = hrefAttrs; - if (download != null) { - supportedProps.download = download; - } - if (rel != null) { - supportedProps.rel = rel; - } - if (typeof target === 'string') { - supportedProps.target = target.charAt(0) !== '_' ? '_' + target : target; + if (props.href != null) { + component = 'a'; + if (hrefAttrs != null) { + const { download, rel, target } = hrefAttrs; + if (download != null) { + supportedProps.download = download; + } + if (rel != null) { + supportedProps.rel = rel; + } + if (typeof target === 'string') { + supportedProps.target = target.charAt(0) !== '_' ? '_' + target : target; + } } } @@ -120,7 +124,7 @@ const View: React.AbstractComponent = supportedProps.ref = setRef; - return createElement('div', supportedProps); + return createElement(component, supportedProps); } ); diff --git a/packages/react-native-web/src/exports/createElement/__tests__/__snapshots__/index-test.js.snap b/packages/react-native-web/src/exports/createElement/__tests__/__snapshots__/index-test.js.snap index bf7ce9c3..def1ea93 100644 --- a/packages/react-native-web/src/exports/createElement/__tests__/__snapshots__/index-test.js.snap +++ b/packages/react-native-web/src/exports/createElement/__tests__/__snapshots__/index-test.js.snap @@ -3,3 +3,11 @@ exports[`exports/createElement renders different DOM elements 1`] = ``; exports[`exports/createElement renders different DOM elements 2`] = `
`; + +exports[`exports/createElement renders different DOM elements 3`] = ` + + + +`; diff --git a/packages/react-native-web/src/exports/createElement/__tests__/index-test.js b/packages/react-native-web/src/exports/createElement/__tests__/index-test.js index c94b02c0..6401a436 100644 --- a/packages/react-native-web/src/exports/createElement/__tests__/index-test.js +++ b/packages/react-native-web/src/exports/createElement/__tests__/index-test.js @@ -18,12 +18,16 @@ describe('exports/createElement', () => { expect(container.firstChild).toMatchSnapshot(); ({ container } = render(createElement('main'))); expect(container.firstChild).toMatchSnapshot(); + ({ container } = render( + createElement('svg', { children: createElement('image', { href: '#href' }) }) + )); + expect(container.firstChild).toMatchSnapshot(); }); describe('prop "accessibilityRole"', () => { test('string component type', () => { const { container } = render(createElement('span', { accessibilityRole: 'link' })); - expect(container.firstChild.nodeName).toBe('A'); + expect(container.firstChild.nodeName).toBe('SPAN'); }); test('function component type', () => { diff --git a/packages/react-native-web/src/modules/AccessibilityUtil/propsToAccessibilityComponent.js b/packages/react-native-web/src/modules/AccessibilityUtil/propsToAccessibilityComponent.js index b0ec62ca..37b829fd 100644 --- a/packages/react-native-web/src/modules/AccessibilityUtil/propsToAccessibilityComponent.js +++ b/packages/react-native-web/src/modules/AccessibilityUtil/propsToAccessibilityComponent.js @@ -21,7 +21,6 @@ const roleComponents = { figure: 'figure', insertion: 'ins', form: 'form', - link: 'a', list: 'ul', listitem: 'li', main: 'main', @@ -38,11 +37,6 @@ const propsToAccessibilityComponent = (props: Object = emptyObject): void | stri return 'label'; } - // special-case for "href" which becomes a link - if (props.href != null) { - return 'a'; - } - const role = propsToAriaRole(props); if (role) { if (role === 'heading') {