diff --git a/src/__tests__/native/components.test.tsx b/src/__tests__/native/components.test.tsx index 3a596038..218b8fe8 100644 --- a/src/__tests__/native/components.test.tsx +++ b/src/__tests__/native/components.test.tsx @@ -1,15 +1,17 @@ import { Button as RNButton, TextInput as RNTextInput, + View as RNView, type ButtonProps, type TextInputProps, + type ViewProps, } from "react-native"; import { render } from "@testing-library/react-native"; import { copyComponentProperties } from "react-native-css/components/copyComponentProperties"; import { TextInput } from "react-native-css/components/TextInput"; import { registerCSS, testID } from "react-native-css/jest"; -import { useCssElement } from "react-native-css/native"; +import { styled, useCssElement } from "react-native-css/native"; import type { StyledConfiguration, StyledProps, @@ -114,3 +116,177 @@ test("nativeStyleMapping with boolean true on custom component", () => { expect(component.props.textAlign).toBe("right"); expect(component.props.style).not.toHaveProperty("textAlign"); }); + +/** + * A component with props of its own for a `target: false` config to write into. + * + * `nativeStyleMapping` is typed against the target component's props, so the defect below is only + * expressible on a component that HAS the props being mapped to — which is also exactly who it + * bites. Every shipped component is either bare-string or `{ target: "style" }`; the one exception, + * `Button`, maps `{ color: "color" }` onto a component that ignores an unknown `style` prop. + */ +interface LabelledProps extends ViewProps { + readonly labelColor?: string; + readonly badgeColor?: string; +} + +const Labelled = (props: LabelledProps) => ; + +test("a target:false config does not take a style-target config's declarations", () => { + // `calculateProps` writes a `target: false` config's declarations into `normal.style`, because + // `rule.target || "style"` turns the `false` into that string, and `nativeStyleMapping` reads + // them back out by the same hardcoded name. The pair is deliberate — the scratch space is how the + // declarations travel — but it is not PRIVATE, so a component carrying both kinds of config puts + // both sets in one place and each drains the other's. + // + // Both configs carry a `color`, so a swap is visible whichever way it goes: the style-target + // config's orange must stay in `style`, and only the `target: false` config's blue may be + // redistributed into `labelColor`. + registerCSS(`.c1 { color: orange; } .c2 { color: blue; }`); + + const Styled = styled(Labelled, { + className: { target: "style" }, + labelClassName: { + target: false, + nativeStyleMapping: { color: "labelColor" }, + }, + }); + + const component = render( + , + ).getByTestId(testID); + + expect(component.props.style).toStrictEqual({ color: "#ffa500" }); + expect(component.props.labelColor).toBe("#00f"); +}); + +test("a target:false config emits no style prop", () => { + // Three places already say this config owns no style prop: `usePassthrough` skips it, + // `deepMergeConfig` returns before writing a target, and web's `useCssElement` resolves a boolean + // target to the source key instead. The shared scratch space shipped one regardless, carrying + // whatever no mapping entry drained. + registerCSS(`.c3 { color: orange; font-size: 20px; }`); + + const Styled = styled(Labelled, { + labelClassName: { + target: false, + nativeStyleMapping: { color: "labelColor" }, + }, + }); + + const component = render( + , + ).getByTestId(testID); + + expect(component.props.labelColor).toBe("#ffa500"); + expect(component.props).not.toHaveProperty("style"); +}); + +test("two target:false configs each keep their own declarations", () => { + // With one scratch key shared by every `target: false` config, a second one drains whatever the + // first left. Keying the scratch on the config's own source makes that unrepresentable rather + // than merely unlikely: a source is a mapping key, so two configs cannot share one. + registerCSS(`.c4 { color: orange; } .c5 { color: blue; }`); + + const Styled = styled(Labelled, { + labelClassName: { + target: false, + nativeStyleMapping: { color: "labelColor" }, + }, + badgeClassName: { + target: false, + nativeStyleMapping: { color: "badgeColor" }, + }, + }); + + const component = render( + , + ).getByTestId(testID); + + expect(component.props.labelColor).toBe("#ffa500"); + expect(component.props.badgeColor).toBe("#00f"); + expect(component.props).not.toHaveProperty("labelClassName"); + expect(component.props).not.toHaveProperty("badgeClassName"); +}); + +test("declaration order does not change what either config gets", () => { + // The same pair as the first test with the configs declared the other way round. The swap it + // guards against is order-independent, so a fix that only worked one way round would pass there + // and fail here. + // + // `.c6` also carries a declaration NO mapping entry drains. It has nowhere to go — the config + // asked for no style prop — so it is dropped. Before this, it rode the shared scratch out as + // `style: { fontSize: 20 }` on a component that had declared it wanted no `style`. + registerCSS( + `.c6 { color: orange; font-size: 20px; } .c7 { background-color: blue; }`, + ); + + const Styled = styled(Labelled, { + labelClassName: { + target: false, + nativeStyleMapping: { color: "labelColor" }, + }, + className: { target: "style" }, + }); + + const component = render( + , + ).getByTestId(testID); + + expect(component.props.labelColor).toBe("#ffa500"); + expect(component.props.style).toStrictEqual({ backgroundColor: "#00f" }); + expect(component.props).not.toHaveProperty("labelClassName"); +}); + +test("the important path gets its own scratch space too", () => { + // `getStyledProps` calls `nativeStyleMapping` twice per config — once for `normal` and once for + // `important` — so the one changed read has two call sites and the other three tests only reach + // the first. An `!important` declaration travels in the important object, where the same shared + // `style` key produced the same drain. + registerCSS(` + .i1 { color: orange !important; } + .i2 { color: blue !important; font-size: 11px !important; } + `); + + const Styled = styled(Labelled, { + className: { target: "style" }, + labelClassName: { + target: false, + nativeStyleMapping: { color: "labelColor" }, + }, + }); + + const component = render( + , + ).getByTestId(testID); + + expect(component.props.style).toStrictEqual({ color: "#ffa500" }); + expect(component.props.labelColor).toBe("#00f"); +}); + +test("a declaration no mapping entry names is dropped, not moved into style", () => { + // The behaviour change this fix carries, pinned rather than left to a PR sentence. A `target: + // false` config owns no style prop, so a declaration its mapping does not name has nowhere to go. + // It used to ride the shared scratch out as a real `style` prop — `transform` here, which is a + // VISIBLE effect applied to a component that declared it wanted no `style` at all. + // + // There is a runtime escape hatch — a dotted destination writes wherever it points, so + // `{ color: "style.color" }` puts the declaration back. It is not asserted here because it does + // not TYPE: `nativeStyleMapping` destinations are constrained to the component's own prop paths, + // and no path reaches inside `style`. A typed caller therefore has no way to keep one. + registerCSS(`.d1 { transform: translateX(10px); color: orange; }`); + + const Dropping = styled(Labelled, { + labelClassName: { + target: false, + nativeStyleMapping: { color: "labelColor" }, + }, + }); + + const dropped = render( + , + ).getByTestId(testID); + + expect(dropped.props.labelColor).toBe("#ffa500"); + expect(dropped.props).not.toHaveProperty("style"); +}); diff --git a/src/native/react/rules.ts b/src/native/react/rules.ts index f85a66f9..1835fb24 100644 --- a/src/native/react/rules.ts +++ b/src/native/react/rules.ts @@ -263,7 +263,22 @@ export function updateRules( */ const getRuleVariation = weakFamily((rule: StyleRule) => { return weakFamily((config: Config): StyleRule => { - return { ...rule, target: config.target }; + // A `target: false` config still needs somewhere to put its declarations before + // `nativeStyleMapping` redistributes them into real props. That scratch space is this config's + // OWN source key rather than `style`, because `style` is a real target another config can own + // and sharing it makes the two drain each other. Sources are `Object.entries(mapping)` keys, so + // no two configs can collide on one. + // + // A config whose TARGET names another config's source is not excluded by the types, only by + // reachability: that mapping already throws in `updateRules` when the source prop is passed, + // which is the same condition a scratch needs to exist at all. + // + // The key never ships. `getStyledProps` strips every consumed source, and a `target: false` + // config is always consumed because `config.source !== config.target` holds for it. + return { + ...rule, + target: config.target === false ? [config.source] : config.target, + }; }); }); diff --git a/src/native/styles/index.ts b/src/native/styles/index.ts index c598fc6b..8ad243da 100644 --- a/src/native/styles/index.ts +++ b/src/native/styles/index.ts @@ -557,7 +557,8 @@ function nativeStyleMapping( if (typeof config.target === "string") { source = props[config.target]; } else if (config.target === false) { - source = props["style"]; + // The scratch space `getRuleVariation` gave this config — its own source key, not `style`. + source = props[config.source]; } else { const tokens = [...config.target]; const lastToken = tokens.pop()!;