diff --git a/__tests__/buttonBadgeSize.test.tsx b/__tests__/buttonBadgeSize.test.tsx new file mode 100644 index 00000000..6f15a6a7 --- /dev/null +++ b/__tests__/buttonBadgeSize.test.tsx @@ -0,0 +1,59 @@ +import { render } from '@testing-library/react'; +import { ReactNode } from 'react'; +import { ReqoreContent, ReqoreLayoutContent, ReqoreTier, ReqoreUIProvider } from '../src'; +import { ButtonBadge } from '../src/components/Button'; + +/** + * A badge spaced from what precedes it sized the gap from its `size`. Rendered without one — + * as `ReqoreTier` did — the gap was `PADDING_FROM_SIZE[undefined] / 1`, i.e. `width: NaNpx`, + * which the browser drops (and React warned about while the spacer still wrote `width`). + */ + +const renderInProvider = (node: ReactNode) => + render( + + + {node} + + + ); + +/** + * Spacers whose size came out as no number: a spacer given a `NaN` width is neither horizontal + * nor vertical, so its space gets no width at all. + */ +const getUnsizedSpacers = (): HTMLElement[] => + Array.from(document.querySelectorAll('.reqore-spacer > div')).filter( + (space) => getComputedStyle(space).width === 'auto' + ); + +const getNaNWarnings = (): unknown[][] => + vi + .mocked(console.error) + .mock.calls.filter((args) => typeof args[0] === 'string' && args[0].includes('NaN')); + +beforeEach(() => { + vi.mocked(console.error).mockClear(); +}); + +test('a badge without a size spaces itself like a normal-size one', () => { + renderInProvider(); + + const space = document.querySelector('.reqore-spacer > div') as HTMLElement; + + expect(space).toBeTruthy(); + // `normal` padding (8px), halved by the horizontal spacer's two sides. + expect(getComputedStyle(space).width).toBe('4px'); + expect(getUnsizedSpacers()).toEqual([]); + expect(getNaNWarnings()).toEqual([]); +}); + +test('a Tier with a badge computes no NaN size', () => { + renderInProvider( + + ); + + expect(document.querySelector('.reqore-button-badge')).toBeTruthy(); + expect(getUnsizedSpacers()).toEqual([]); + expect(getNaNWarnings()).toEqual([]); +}); diff --git a/__tests__/domPropLeaks.test.tsx b/__tests__/domPropLeaks.test.tsx new file mode 100644 index 00000000..13a6162b --- /dev/null +++ b/__tests__/domPropLeaks.test.tsx @@ -0,0 +1,456 @@ +import isPropValid from '@emotion/is-prop-valid'; +import { render } from '@testing-library/react'; +import { ReactElement } from 'react'; +import { + DatePicker, + ReqoreButton, + ReqoreCheckbox, + ReqoreCollection, + ReqoreContent, + ReqoreControlGroup, + ReqoreHeader, + ReqoreInput, + ReqoreKeyValueTable, + ReqoreLayoutContent, + ReqoreLink, + ReqoreMenu, + ReqoreMenuItem, + ReqoreMessage, + ReqoreNavbarGroup, + ReqoreNavbarItem, + ReqoreP, + ReqorePanel, + ReqorePopover, + ReqoreSlider, + ReqoreSpacer, + ReqoreTabs, + ReqoreTabsContent, + ReqoreTag, + ReqoreTimeline, + ReqoreTree, + ReqoreUIProvider, +} from '../src'; +import { REQORE_CONTROL_GROUP_CHILD_PROPS } from '../src/helpers/styled'; + +/** + * Styling props must not reach the DOM. + * + * Reqore styles its components with props (`fill`, `wrap`, `color`, `opacity`, `theme`, …), and + * styled-components decides which of them the rendered element gets. A DOM tag only gets what + * looks like an HTML attribute, which still lets through the names that ARE attributes somewhere + * (`fill`, `wrap`, `color`, `width`, `direction` …); a component given as `as` (`animated.div`, + * re-resizable, react-aria's `DateInput`, a router link) gets everything and writes it onto its + * own element. React then warns ("React does not recognize the `hasIcon` prop", "Received `true` + * for a non-boolean attribute `fill`") or silently writes `theme="[object Object]"`. + * + * React only warns about a prop name ONCE per page, so a console spy cannot tell the second leak + * of `fill` from the first. This test reads what React actually handed each element instead: + * React 18 keeps an element's current props on the node, under a `__reactProps$` key. + */ + +/** Attributes that exist, but not on the HTML elements a Reqore component renders. */ +const NOT_ON_HTML: Record = { + // SVG presentation attributes. + fill: [], + opacity: [], + direction: [], + offset: [], + spacing: [], + // Legacy `` only. + color: [], + wrap: ['textarea'], + width: ['img', 'canvas', 'video', 'iframe', 'input', 'embed', 'object', 'col', 'colgroup'], + height: ['img', 'canvas', 'video', 'iframe', 'input', 'embed', 'object'], + type: [ + 'input', + 'button', + 'ol', + 'li', + 'a', + 'source', + 'embed', + 'object', + 'script', + 'style', + 'link', + ], + value: [ + 'input', + 'button', + 'select', + 'textarea', + 'option', + 'li', + 'meter', + 'progress', + 'param', + 'data', + 'output', + ], +}; + +const SVG_NAMESPACE = 'http://www.w3.org/2000/svg'; + +const getReactProps = (element: Element): Record | undefined => { + const key = Object.keys(element).find((name) => name.startsWith('__reactProps$')); + + return key ? (element as unknown as Record>)[key] : undefined; +}; + +const describeElement = (element: Element): string => { + const reqoreClass = Array.from(element.classList).find((name) => name.startsWith('reqore-')); + + return `<${element.tagName.toLowerCase()}${reqoreClass ? `.${reqoreClass}` : ''}>`; +}; + +/** Every non-DOM prop React was handed for an element of the document, as ` prop`. */ +const findLeakedProps = (): string[] => { + const leaks: string[] = []; + + document.body.querySelectorAll('*').forEach((element) => { + const props = getReactProps(element); + + if (!props || element.namespaceURI === SVG_NAMESPACE) { + return; + } + + const tag = element.tagName.toLowerCase(); + + Object.entries(props).forEach(([prop, value]) => { + // React renders nothing for an absent value, and says nothing about it either. + if (value === undefined || value === null) { + return; + } + + if (prop === 'children' || prop.startsWith('data-') || prop.startsWith('aria-')) { + return; + } + + const allowedOn = NOT_ON_HTML[prop]; + + if (!isPropValid(prop) || (allowedOn && !allowedOn.includes(tag))) { + leaks.push(`${describeElement(element)} ${prop}`); + } + }); + }); + + return [...new Set(leaks)].sort(); +}; + +const REACT_DOM_PROP_WARNINGS = [ + /does not recognize the `/, + /for a non-boolean attribute/, + /Invalid value for prop/, + /Unknown event handler property/, + /Received NaN for the/, +]; + +const formatConsoleCall = ([format, ...args]: unknown[]): string => { + let index = 0; + + return typeof format === 'string' + ? format.replace(/%s/g, () => String(args[index++])) + : String(format); +}; + +const getReactDomPropWarnings = (): string[] => + vi + .mocked(console.error) + .mock.calls.map(formatConsoleCall) + .filter((message) => REACT_DOM_PROP_WARNINGS.some((pattern) => pattern.test(message))); + +const renderInProvider = (element: ReactElement) => + render( + + + {element} + + + ); + +/** A router's link: it writes every prop it does not know onto its ``. */ +const RouterLinkStub = ({ to, children, ...rest }: any) => ( + + {children} + +); + +// `fill` is not a checkbox prop, but a containing control group hands it to every child, and +// consumers pass it the same way. +const FILL = { fill: true } as any; + +const CASES: [string, () => ReactElement][] = [ + ['a bare DatePicker', () => ], + [ + 'a minimal pill DatePicker with time', + () => , + ], + [ + 'filling Tabs', + () => ( + + One + Two + + ), + ], + [ + 'a filling Collection with a height', + () => ( + + ), + ], + ['a Checkbox given `fill`', () => ], + [ + 'a filling ControlGroup of Tag, Paragraph, Input, Checkbox and Button', + () => ( + + + Paragraph + + + + Button + + + ), + ], + [ + 'a Slider in a filling ControlGroup', + () => ( + + + + ), + ], + [ + 'a resizable Menu with a width', + () => ( + + + + ), + ], + [ + 'a Link rendered as a router link', + () => ( + + Go to issues + + ), + ], + [ + 'a Message with a tooltip', + () => ( + + Message + + ), + ], + [ + 'an open Popover whose content is a DOM element', + () => ( + Text} + > + Open + + ), + ], + [ + 'an open Popover whose content is a Message', + () => ( + Message} + > + Open + + ), + ], + [ + 'a Timeline with tooltips', + () => ( + + ), + ], + [ + 'a Navbar', + () => ( + + + Item + + + ), + ], + ['a Tree given a `mode`', () => ], + [ + 'a Panel with an opacity and bottom actions', + () => ( + + Content + + ), + ], + [ + 'a resizable, rounded, interactive Panel', + () => ( + + Content + + ), + ], + ['a KeyValueTable', () => ], + ['a Spacer', () => ], +]; + +describe('no styling prop reaches the DOM', () => { + const originalResizeObserver = globalThis.ResizeObserver; + + beforeAll(() => { + // jsdom has none; the slider measures its thumbs with one. + globalThis.ResizeObserver = class { + observe() {} + unobserve() {} + disconnect() {} + } as unknown as typeof ResizeObserver; + }); + + afterAll(() => { + globalThis.ResizeObserver = originalResizeObserver; + }); + + beforeEach(() => { + vi.mocked(console.error).mockClear(); + }); + + test.each(CASES)('%s', (_name, element) => { + renderInProvider(element()); + + expect(findLeakedProps()).toEqual([]); + expect(getReactDomPropWarnings()).toEqual([]); + }); +}); + +describe('the leak detector', () => { + // Without these, a detector that found nothing would pass every case above. + test('finds a styling flag a DOM element was given', () => { + renderInProvider( +
+ ); + + expect(findLeakedProps()).toEqual([ + ' fill', + ' hasIcon', + ' width', + ]); + }); + + test('leaves real attributes, data and aria attributes, and SVG presentation alone', () => { + renderInProvider( +
+