fix: prefer writable appearance proxy members

DjDeveloperr · DjDeveloperr · commit 7eae675db926 · 2026-06-29T16:06:05.000-04:00
diff --git a/HANDOFF.md b/HANDOFF.md
@@ -111,6 +111,44 @@ During this thread `agent-device` was version `0.18.0`; the npm package is
 
 ## Current Verified State
 
+Update from 2026-06-29 16:05 EDT:
+
+- Simulator-only rule remains active. Do not use physical iPhone/iPad devices.
+- GitHub Actions run `28396473566` on `1b12a71e` kept setup, dependency
+  install, FFI boundary check, V8 download, libffi build, metadata generation,
+  NativeScript build, CLI build, and macOS tests green.
+- iOS simulator still failed in `ApiTests.js Appearance` at the same
+  `UILabel.appearance().textColor` readback:
+  `Expected undefined to be { }`.
+- Root cause refinement:
+  - The previous host-set fallback still did not help if the returned
+    `UIAppearance` proxy got a getter-only own accessor.
+  - The accessor installer de-duplicated target-class metadata by the first
+    property name it saw. If duplicate members exist and the first member is
+    readonly or less capable than a later UIKit property member, assignment is
+    ignored before the host-object fallback can run.
+- Current runtime fix:
+  - UIAppearance accessor installation now gathers candidate members by
+    property name and prefers writable members, then explicit setter selectors,
+    before defining the cache-backed own accessor.
+  - Tagged UIAppearance host-object assignment now runs before generic
+    native-object assignment and uses the same writable-preferred proxy member
+    selection.
+  - Source coverage guards both the writable-preferred accessor selection and
+    the UIAppearance-before-generic host-set order.
+- Local verification after this correction:
+  - `node packages/react-native/test/runtime-objc-property-setter.test.js`
+    passed.
+  - Runtime RN JS tests passed:
+    `for f in packages/react-native/test/*.test.js; do node "$f" || exit 1; done`.
+  - `npm run check:ffi-boundaries` passed.
+  - `git diff --check` passed.
+  - `npm run build:macos-cli` passed.
+- Not done:
+  - Commit/push this writable-member selection correction and watch fresh PR
+    CI.
+  - If CI turns green, resume the dedicated simulator-only RNS parity sweep.
+
 Update from 2026-06-29 15:13 EDT:
 
 - Simulator-only rule remains active. Do not use physical iPhone/iPad devices.
diff --git a/NativeScript/ffi/shared/bridge/HostObjects.mm b/NativeScript/ffi/shared/bridge/HostObjects.mm
@@ -644,6 +644,41 @@ void cacheAppearanceProxyPropertyValue(
   }
 }
 
+const NativeApiMember* betterAppearanceProxyAccessorMember(
+    const NativeApiMember* current, const NativeApiMember& candidate) {
+  if (current == nullptr) {
+    return &candidate;
+  }
+  if (current->readonly != candidate.readonly) {
+    return candidate.readonly ? current : &candidate;
+  }
+  if (current->setterSelectorName.empty() &&
+      !candidate.setterSelectorName.empty()) {
+    return &candidate;
+  }
+  if (current->signatureOffset == MD_SECTION_OFFSET_NULL &&
+      candidate.signatureOffset != MD_SECTION_OFFSET_NULL) {
+    return &candidate;
+  }
+  return current;
+}
+
+const NativeApiMember* selectAppearanceProxyPropertyMember(
+    const std::vector<NativeApiMember>& members, const std::string& property) {
+  const NativeApiMember* selected = nullptr;
+  for (const auto& member : members) {
+    if (!member.property || member.name != property) {
+      continue;
+    }
+    bool memberIsStatic = (member.flags & metagen::mdMemberStatic) != 0;
+    if (memberIsStatic) {
+      continue;
+    }
+    selected = betterAppearanceProxyAccessorMember(selected, member);
+  }
+  return selected;
+}
+
 class NativeApiSuperHostObject final : public HostObject {
  public:
   NativeApiSuperHostObject(std::shared_ptr<NativeApiBridge> bridge,
@@ -1787,34 +1822,13 @@ NativeApiHostSetResult set(Runtime& runtime, const PropNameID& name, const Value
       throw JSError(runtime, "Cannot set property on nil object.");
     }
 
-    if (const NativeApiSymbol* symbol =
-            bridge_->findClassForRuntimeClass(object_getClass(object_))) {
-      const auto& members = bridge_->membersForClass(*symbol);
-      if (const NativeApiMember* propertyMember =
-              selectWritablePropertyMember(members, property, false)) {
-        if (propertyMember->readonly) {
-          throw JSError(
-              runtime, "Attempted to assign to readonly property.");
-        }
-        NativeApiMember setterMember = *propertyMember;
-        setterMember.selectorName = propertyMember->setterSelectorName;
-        setterMember.signatureOffset = propertyMember->setterSignatureOffset;
-        Value args[] = {Value(runtime, value)};
-        callObjCSelector(runtime, bridge_, object_, false,
-                         setterMember.selectorName, &setterMember, args, 1);
-        cacheAppearanceProxyPropertyValue(runtime, bridge_, object_, property,
-                                          value);
-        NATIVE_API_SET_RETURN(true);
-      }
-    }
-
     if (Class appearanceClass =
             taggedAppearanceProxyClass(runtime, bridge_, object_)) {
       if (const NativeApiSymbol* symbol =
               bridge_->findClassForRuntimeClass(appearanceClass)) {
         const auto& members = bridge_->membersForClass(*symbol);
         if (const NativeApiMember* propertyMember =
-                selectWritablePropertyMember(members, property, false)) {
+                selectAppearanceProxyPropertyMember(members, property)) {
           if (propertyMember->readonly) {
             throw JSError(
                 runtime, "Attempted to assign to readonly property.");
@@ -1841,6 +1855,27 @@ throw JSError(
       }
     }
 
+    if (const NativeApiSymbol* symbol =
+            bridge_->findClassForRuntimeClass(object_getClass(object_))) {
+      const auto& members = bridge_->membersForClass(*symbol);
+      if (const NativeApiMember* propertyMember =
+              selectWritablePropertyMember(members, property, false)) {
+        if (propertyMember->readonly) {
+          throw JSError(
+              runtime, "Attempted to assign to readonly property.");
+        }
+        NativeApiMember setterMember = *propertyMember;
+        setterMember.selectorName = propertyMember->setterSelectorName;
+        setterMember.signatureOffset = propertyMember->setterSignatureOffset;
+        Value args[] = {Value(runtime, value)};
+        callObjCSelector(runtime, bridge_, object_, false,
+                         setterMember.selectorName, &setterMember, args, 1);
+        cacheAppearanceProxyPropertyValue(runtime, bridge_, object_, property,
+                                          value);
+        NATIVE_API_SET_RETURN(true);
+      }
+    }
+
     if (auto setterSelectorName =
             runtimeWritablePropertySetter(object_, property)) {
       Value args[] = {Value(runtime, value)};
@@ -2013,15 +2048,20 @@ void installAppearanceProxyPropertyAccessors(
       runtime.global().getPropertyAsObject(runtime, "Object");
   Function defineProperty =
       objectConstructor.getPropertyAsFunction(runtime, "defineProperty");
-  std::unordered_set<std::string> installed;
   std::shared_ptr<void> retainedNative =
       retainAppearanceProxyForAccessor(native);
   const auto& members = bridge->membersForClass(*symbol);
+  std::unordered_map<std::string, const NativeApiMember*> accessors;
   for (const auto& member : members) {
-    if (!shouldInstallAppearanceProxyAccessor(member) ||
-        !installed.insert(member.name).second) {
+    if (!shouldInstallAppearanceProxyAccessor(member)) {
       continue;
     }
+    accessors[member.name] =
+        betterAppearanceProxyAccessorMember(accessors[member.name], member);
+  }
+
+  for (const auto& accessor : accessors) {
+    const NativeApiMember& member = *accessor.second;
 
     try {
       Object descriptor(runtime);
diff --git a/PROGRESS.md b/PROGRESS.md
@@ -17,6 +17,45 @@ TypeScript/UI worklets.
 
 ## Latest Update - 2026-06-29
 
+### 2026-06-29 16:05 EDT - UIAppearance writable member selection
+
+- Goal:
+  - Stay on PR #46's generic runtime primitive path for the RN module branch
+    and remain simulator-only. No physical devices were used.
+- CI finding:
+  - GitHub Actions run `28396473566` on `1b12a71e` kept setup, dependency
+    install, FFI boundary check, V8 download, libffi build, metadata
+    generation, NativeScript build, CLI build, and macOS tests green.
+  - iOS simulator still failed in `ApiTests.js Appearance` at the same
+    `UILabel.appearance().textColor` readback:
+    `Expected undefined to be { }`.
+  - Refinement: if target-class metadata contains duplicate property members,
+    the appearance accessor installer could de-duplicate on the first member
+    name and install a getter-only descriptor before the writable UIKit member
+    was seen. That drops assignment before the host-object fallback can cache
+    the value.
+- Changes:
+  - UIAppearance accessor installation now gathers members by property name
+    first and prefers writable members, then members with explicit setter
+    selectors, before defining the cache-backed own accessor.
+  - Tagged UIAppearance host-object assignment now runs before generic
+    native-object assignment and uses the same writable-preferred proxy member
+    selection.
+  - Source coverage now guards the writable-preferred accessor selection and
+    the UIAppearance-before-generic host-set order.
+- Verification:
+  - `node packages/react-native/test/runtime-objc-property-setter.test.js`
+    passed.
+  - Runtime RN JS tests passed:
+    `for f in packages/react-native/test/*.test.js; do node "$f" || exit 1; done`.
+  - `npm run check:ffi-boundaries` passed.
+  - `git diff --check` passed.
+  - `npm run build:macos-cli` passed and compiled/linked the edited V8 bridge.
+- Still next:
+  - Commit/push this writable-member selection correction and watch fresh PR
+    CI for the authoritative iOS simulator result.
+  - If CI turns green, resume the dedicated simulator-only RNS parity sweep.
+
 ### 2026-06-29 15:13 EDT - UIAppearance host-set fallback
 
 - Goal:
diff --git a/packages/react-native/test/runtime-objc-property-setter.test.js b/packages/react-native/test/runtime-objc-property-setter.test.js
@@ -51,8 +51,12 @@ const appearanceAccessorSource = runtimeHostObjects.slice(
   appearanceAccessorStart,
   appearanceAccessorEnd,
 );
+const nativeObjectHostObjectStart = runtimeHostObjects.indexOf(
+  "class NativeApiObjectHostObject final",
+);
 const appearanceHostSetStart = runtimeHostObjects.indexOf(
-  "if (Class appearanceClass =",
+  "NativeApiHostSetResult set(Runtime& runtime, const PropNameID& name, const Value& value) override",
+  nativeObjectHostObjectStart,
 );
 const appearanceHostSetEnd = runtimeHostObjects.indexOf(
   "\n    if (auto setterSelectorName =",
@@ -78,16 +82,23 @@ assert(
   runtimeHostObjects.includes("installAppearanceProxyPropertyAccessors") &&
     runtimeHostObjects.includes("makeAppearanceProxyPropertySetter") &&
     runtimeHostObjects.includes("makeAppearanceProxyPropertyGetter") &&
+    runtimeHostObjects.includes("betterAppearanceProxyAccessorMember") &&
+    runtimeHostObjects.includes("std::unordered_map<std::string, const NativeApiMember*> accessors") &&
     runtimeHostObjects.includes("cacheAppearanceProxyPropertyValue(") &&
     appearanceAccessorSource.includes("runtimeWritablePropertySetter(native, member.name)") &&
     appearanceAccessorSource.includes("if (!member.readonly)") &&
+    appearanceAccessorSource.includes("betterAppearanceProxyAccessorMember(accessors[member.name], member)") &&
+    !appearanceAccessorSource.includes("std::unordered_set<std::string> installed") &&
     !appearanceAccessorSource.includes("!member.readonly && !member.setterSelectorName.empty()"),
-  "runtime UIAppearance proxies should install safe property accessors backed by the appearance cache",
+  "runtime UIAppearance proxies should install writable-preferred safe property accessors backed by the appearance cache",
 );
 assert(
   appearanceHostSetSource.includes("runtimeWritablePropertySetter(object_, property)") &&
+    appearanceHostSetSource.includes("selectAppearanceProxyPropertyMember(members, property)") &&
+    appearanceHostSetSource.indexOf("taggedAppearanceProxyClass(runtime, bridge_, object_)") <
+      appearanceHostSetSource.indexOf("findClassForRuntimeClass(object_getClass(object_))") &&
     !appearanceHostSetSource.includes("propertyMember->readonly ||\n              propertyMember->setterSelectorName.empty()"),
-  "runtime UIAppearance host-object assignment should use the same runtime setter fallback before caching",
+  "runtime UIAppearance host-object assignment should select proxy members before generic object setters and use the same runtime setter fallback before caching",
 );
 assert(
   !runtimeHostObjects.includes("SetNativeApiObjectPrototype(runtime, resultObject"),
