Skip to content

fix: rules which set variables causing remounts #100

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

Merged
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
39 changes: 39 additions & 0 deletions src/runtime/native/__tests__/container-queries.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,45 @@ import { registerCSS } from "react-native-css/jest";
const parentID = "parent";
const childID = "child";

test("Unnamed containers", () => {
registerCSS(`
:root, :host {
--color-white: #fff;
}
.\\@container {
container-type: inline-size;
}
.\\@sm\\:text-white {
@container (width >= 24rem) {
color: var(--color-white);
}
}
`);

render(
<View testID={parentID} className="@container">
<View testID={childID} className="@sm:text-white" />
</View>,
);

const parent = screen.getByTestId(parentID);
const child = screen.getByTestId(childID);

expect(child).toHaveStyle(undefined);

// Jest does not fire layout events, so we need to manually
fireEvent(parent, "layout", {
nativeEvent: {
layout: {
width: 500,
height: 200,
},
},
});

expect(child).toHaveStyle({ color: "#fff" });
});

test("container query width", () => {
registerCSS(`
.container {
Expand Down
18 changes: 9 additions & 9 deletions src/runtime/native/__tests__/grouping.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,24 @@ const childID = "child";
jest.useFakeTimers();

test("groups", () => {
registerCSS(
`.group\\/item .my-class {
registerCSS(`
.group\\/item .my-class {
color: red;
}`,
);
}
`);

const { rerender, getByTestId } = render(
<View testID={parentID} className="group/item">
render(
<View testID={parentID} className="group/item will-change-container">
<View testID={childID} className="my-class" />
</View>,
);

const component = getByTestId(childID);
const component = screen.getByTestId(childID);

expect(component.props.style).toStrictEqual({ color: "#f00" });

rerender(
<View testID={parentID}>
screen.rerender(
<View testID={parentID} className="will-change-container">
<View testID={childID} className="my-class" />
</View>,
);
Expand Down
11 changes: 4 additions & 7 deletions src/runtime/native/__tests__/upgrading.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ test("adding a group", () => {

expect(log.mock.calls).toEqual([
[
"ReactNativeCss: className 'group' added a container after the initial render. This causes the components state to be reset and all children be re-mounted. This will cause unexpected behavior. Use the className 'will-change-container' to avoid this warning. If this was caused by sibling components being added/removed, use a 'key' prop so React can track the component correctly.",
"ReactNativeCss: className 'group' added or removed a container after the initial render. This causes the components state to be reset and all children be re-mounted. This will cause unexpected behavior. Use the className 'will-change-container' to avoid this warning. If this was caused by sibling components being added/removed, use a 'key' prop so React can track the component correctly.",
],
]);
});

test.only("will-change-container", () => {
test("will-change-container", () => {
registerCSS(
`.group .my-class {
color: red;
Expand All @@ -69,9 +69,6 @@ test.only("will-change-container", () => {
</View>,
);

expect(log.mock.calls).toEqual([
[
"ReactNativeCss: className 'group' added or removed a container after the initial render. This causes the components state to be reset and all children be re-mounted. This will cause unexpected behavior. Use the className 'will-change-container' to avoid this warning. If this was caused by sibling components being added/removed, use a 'key' prop so React can track the component correctly.",
],
]);
// There shouldn't be any error, as we continued to have a container
expect(log.mock.calls).toEqual([]);
});
61 changes: 31 additions & 30 deletions src/runtime/native/injection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type {
ReactNativeCssStyleSheet,
StyleRuleSet,
} from "../../compiler";
import { DEFAULT_CONTAINER_NAME } from "./conditions/container-query";
import {
family,
observable,
Expand All @@ -27,6 +28,36 @@ StyleCollection.keyframes = family<string, Observable<Animation_V2[1]>>(() => {
StyleCollection.inject = function (options: ReactNativeCssStyleSheet) {
observableBatch.current = new Set();

StyleCollection.styles("will-change-variable").set([
{
s: [0],
v: [],
},
]);

StyleCollection.styles("will-change-container").set([
{
s: [0],
c: [DEFAULT_CONTAINER_NAME],
},
]);

StyleCollection.styles("will-change-animation").set([
{
s: [0],
a: true,
},
]);

StyleCollection.styles("will-change-pressable").set([
{
s: [0],
p: {
h: 1,
},
},
]);

if (options.s) {
for (const style of options.s) {
StyleCollection.styles(style[0]).set(style[1]);
Expand Down Expand Up @@ -99,33 +130,3 @@ function isDeepEqual(a: unknown, b: unknown): boolean {

return true;
}

StyleCollection.styles("will-change-variable").set([
{
s: [0],
v: [],
},
]);

StyleCollection.styles("will-change-container").set([
{
s: [0],
c: [],
},
]);

StyleCollection.styles("will-change-animation").set([
{
s: [0],
a: true,
},
]);

StyleCollection.styles("will-change-pressable").set([
{
s: [0],
p: {
h: 1,
},
},
]);
19 changes: 14 additions & 5 deletions src/runtime/native/react/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
hoverFamily,
VAR_SYMBOL,
weakFamily,
type ContainerContextValue,
type VariableContextValue,
} from "../reactivity";
import { stylesFamily } from "../styles";
Expand All @@ -36,8 +37,8 @@ export function updateRules(

let usesVariables = false;

let variables = state.variables ? inheritedVariables : undefined;
let containers = state.containers ? inheritedContainers : undefined;
let variables: VariableContextValue | undefined;
let containers: ContainerContextValue | undefined;
const inlineVariables = new Set<InlineVariable>();

let animated = false;
Expand Down Expand Up @@ -101,6 +102,16 @@ export function updateRules(

usesVariables ||= Boolean(rule.dv);

// We do this even if the rule doesn't match so we can maintain a consistent render tree
// We we need to inject React context
if (rule.v) {
variables ??= inheritedVariables;
}

if (rule.c) {
containers ??= inheritedContainers;
}

if (
!testRule(
rule,
Expand All @@ -114,11 +125,8 @@ export function updateRules(
}

if (rule.v) {
// We're going to set a value, so we need to create a new object
if (variables === inheritedVariables) {
variables = { ...inheritedVariables };
} else {
variables ??= { ...inheritedVariables };
}

for (const v of rule.v) {
Expand Down Expand Up @@ -199,6 +207,7 @@ export function updateRules(
guards,
animated,
pressable,
variables,
};
}

Expand Down
Loading