Skip to content

2.0.0-beta.15: style object binding can't remove explicit-undefined properties and destructively mutates user style objects #2828

Description

@yumemi-thomas

Describe the bug

The runtime style() differ — used whenever the style value is not an inline object literal (a computed object, a constant, a ternary between objects, a prop, or the spread path) — has two defects producing three user-visible symptoms:

  1. Explicit undefined never removes a property. style={boxStyle()} with boxStyle = () => ({ color: on() ? "red" : undefined }) leaves color: red applied forever: the differ calls setProperty(name, undefined), which is a CSSOM no-op. (The inline-literal form style={{ color: on() ? "red" : undefined }} happens to work because the compiler decomposes it into setStyleProperty calls, which do handle removal — so the bug surfaces exactly when the object comes from anywhere else.)
  2. The differ mutates the user's previous style object with delete prev[s] (1.x diffed against an internal copy). Toggling between two constant objects A/B permanently deletes keys from them, so styles are silently lost on flip-back: A = { color: "red", "background-color": "yellow" } loses color on the first flip to B, and the element never gets red back.
  3. Corollary of 2: when the differ re-runs with value === prev — which happens whenever any other dynamic binding is grouped into the same compiled template effect, i.e. the exact setup of Using imported object for style attribute is reevaluated if other attribute uses updated signal #1938 — the delete prev[s] loop deletes every key of the object while iterating it, emptying the user's constant object in one pass without touching the DOM. All its styles are then lost on the next real update.

Related to open #1938: same style() function and same trigger shape (another binding's signal re-runs the shared effect), but in 1.x the consequence was only a spurious re-evaluation — in 2.0 it destroys the user's object (symptom 3).

All three symptoms are regressions from 1.x, verified on 1.9.14: explicit undefined removes the property, and toggling/shared-effect re-runs never mutate the user's objects (1.x style() accumulates the applied declarations in an internal object it returns and threads back as prev; the user's values are only ever read).

Your Example Website or App

https://stackblitz.com/edit/solidjs-templates-e5b4bufb?file=src%2FApp.tsx

import { createSignal, flush } from "solid-js";

// constant style objects, defined once (shared/imported styles pattern)
const A = { color: "red", "background-color": "yellow" };
const B = { color: "green" };

// (1) explicit `undefined` value never removes the property
function Box1() {
  const [on, setOn] = createSignal(true);
  const [report, setReport] = createSignal("");
  let box!: HTMLDivElement;
  // computed style object — hits the runtime style() differ
  // (an inline literal is decomposed by the compiler and unaffected)
  const boxStyle = () => ({ color: on() ? "red" : undefined });
  return (
    <section>
      <div ref={box} style={boxStyle()}>box1</div>
      <button
        onClick={() => {
          setOn(!on());
          flush();
          setReport(`on=${on()} — inline color: "${box.style.color}"`);
        }}
      >
        toggle box1 color
      </button>
      <pre>{report()}</pre>
    </section>
  );
}

// (2) the differ mutates the constant objects; styles lost on flip-back
function Box2() {
  const [useB, setUseB] = createSignal(false);
  const [report, setReport] = createSignal("");
  let box!: HTMLDivElement;
  return (
    <section>
      <div ref={box} style={useB() ? B : A}>box2</div>
      <button
        onClick={() => {
          setUseB(!useB());
          flush();
          setReport(
            `using ${useB() ? "B" : "A"} — inline color: "${box.style.color}", ` +
              `background: "${box.style.backgroundColor}"\nA = ${JSON.stringify(A)}`
          );
        }}
      >
        toggle box2 A/B
      </button>
      <pre>{report()}</pre>
    </section>
  );
}

// (3) any other dynamic binding in the same template effect empties a
//     constant style object without touching the DOM
function Box3() {
  const C = { color: "red", "background-color": "yellow" };
  const [n, setN] = createSignal(0);
  const [report, setReport] = createSignal("");
  return (
    <section>
      <div title={`count ${n()}`} style={C}>box3</div>
      <button
        onClick={() => {
          setN(n() + 1);
          flush();
          setReport(`C is now ${JSON.stringify(C)}`);
        }}
      >
        bump box3 title attr
      </button>
      <pre>{report()}</pre>
    </section>
  );
}

export default function App() {
  return (
    <>
      <Box1 />
      <Box2 />
      <Box3 />
    </>
  );
}

Steps to Reproduce the Bug or Issue

  1. On load, box1 is red (cssText = color: red;), box2 is red on yellow.
  2. Click "toggle box1 color". Actual — the color is never removed:
on=false — inline color: "red"
  1. Click "toggle box2 A/B" (A → B). The DOM looks right at this point, but constant A has already lost its color key:
using B — inline color: "green", background: ""
A = {"background-color":"yellow"}
  1. Click "toggle box2 A/B" again (B → A). Actual — red is gone forever; box2 is now just a yellow box:
using A — inline color: "", background: "yellow"
A = {"background-color":"yellow"}
  1. Click "bump box3 title attr" (only the title attribute's signal changes). Actual — the constant style object is emptied in place:
C is now {}

Expected behavior

Diffing must treat a nullish value as removal and must never write to user-owned objects:

step 2: on=false — inline color: ""                                  (property removed)
step 3: using B — inline color: "green", background: ""
        A = {"color":"red","background-color":"yellow"}              (A untouched)
step 4: using A — inline color: "red", background: "yellow"          (flip-back restores A fully)
step 5: C is now {"color":"red","background-color":"yellow"}         (C untouched)

Screenshots or Videos

No response

Platform

  • OS: macOS
  • Browser: Chrome
  • Version: 2.0.0-beta.15 (verified at next HEAD bad66625; 1.x comparison on solid-js 1.9.14)

Additional context

Root cause is style() in node_modules/dom-expressions/src/client.js:247-263 (source repo: ryansolid/dom-expressions, src/client.js); the same function serves the spread() path:

export function style(node, value, prev) {
  ...
  prev || (prev = {});
  let v, s;
  for (s in value) {
    v = value[s];
    if (v !== prev[s]) nodeStyle.setProperty(s, v);   // ← undefined: CSSOM no-op (symptom 1)
    delete prev[s];                                   // ← mutates the USER's previous object (symptoms 2+3)
  }
  for (s in prev) value[s] == null && nodeStyle.removeProperty(s);
}

prev is the raw previous value the user passed (the compiled effect threads it through), not an internal copy as in 1.x. When value === prev (shared-effect re-run, symptom 3), delete prev[s] deletes the keys of the object being iterated, emptying it.

Suggested fix direction: diff against a private shallow copy of the previously applied declarations (kept in the effect state or on the element) instead of the user's object; route nullish new values through removeProperty in the first loop; never delete from, or otherwise write to, the incoming objects.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions