Skip to content

fix(Helpers): valueless field objects read as empty, not raw arrays#68

Merged
parisek merged 3 commits into
mainfrom
fix/field-formatter-valueless-fields
Jul 7, 2026
Merged

fix(Helpers): valueless field objects read as empty, not raw arrays#68
parisek merged 3 commits into
mainfrom
fix/field-formatter-valueless-fields

Conversation

@parisek

@parisek parisek commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Summary

Helpers::fieldFormatter() leaked raw ACF field-definition arrays into Twig templates for fields that were surfaced without a saved value. Templates piping such a value through a string filter (|typography, |e, …) fatalled with TypographyExtension::applyTypography(): Argument #1 ($string) must be of type Stringable|string|null, array given — and because StarterBase::render_namespaced_twig_template() catches any \Throwable and renders the alert fallback, the visible symptom was a completely misleading "Component template X.twig not found".

How it was found (real-world repro)

Discovered on mairateam during the doc-efficiency alignment (mairateam#37), immediately after upgrading parisek/timber-kit 1.7.7 → 1.16.0:

  1. Homepage showed "Component template header.twig not found" — actually a missing _xt() (fixed by the upgrade itself; the helpers ship since 1.8).
  2. The upgrade then broke all 10 author pages with "Component template contact-image.twig not found". Instrumenting the fallback catch revealed the real exception above, thrown at contact-image.twig:118{{ content.form_title|typography }} received an array.

Root cause chain

  1. Since the options local-store walk (getFieldObjectsForOptions(), 1.8+), formatFields('option') surfaces options-page groups that older versions never returned — including groups whose fields have no saved value in the DB (typical on dev/staging databases, fresh installs, or newly added option groups).
  2. Such field objects arrive from ACF either without a value key entirely, or with an explicit value => null.
  3. The old guard:
    if ( ! isset( $field['type'] ) || ! isset( $field['value'] ) ) {
        return $field;   // ← returns the ENTIRE ACF field-definition array
    }
    isset() is false in both cases, so the raw definition (key, label, sub_fields, conditional_logic, _valid, …) flowed through mapFields() (non-empty → kept) into $context['content'] — where template isset/truthiness guards happily pass an array and string filters fatal.
  4. On ≤1.7.x these fields simply never surfaced for options pages, so downstream code (e.g. mairateam Base.php: isset( $global_fields['form_title'] ) ? … : '') was written against "absent when unfilled" semantics. 1.16 silently changed that to "present as raw definition array".

The fix — decision table

Input Before After
$field empty / not array FALSE FALSE (unchanged)
No type key pass-through $field pass-through (unchanged — non-ACF arrays)
value key missing (any type) raw field-definition array FALSEmapFields() drops the key; template isset guards behave exactly like pre-1.8
value => null, scalar/group/… raw field-definition array FALSE
value => null, repeater / flexible_content pass-through $field pass-through (unchanged — documented block-preview contract, covered by existing tests test_repeater_null_value_returns_field_as_is + test_flexible_content_null_value_returns_field_as_is)
value present & non-null type-specific formatting unchanged

Callers verified:

  • mapFields() drops FALSE via its ! empty( $value ) check → key absent, matching pre-1.8 behaviour.
  • Nested recursion (group/repeater/flexible sub-fields) assigns FALSE into the nested value rather than pruning the key — safer than leaking arrays and consistent with existing nested behaviour.

Review trail

Independent Codex review ran over the first iteration and found P2: the repeater/flexible exemption used !isset() alone, so those two types with a missing value key still leaked raw definitions. Fixed in the follow-up commit: array_key_exists( 'value', $field ) now draws the line — missing key = empty for every type; the null pass-through applies only to an explicit value => null on repeater/flexible. Codex also confirmed: guard order is sound for every type branch below it, no legitimate top-level group value => null flow needs the old pass-through, and top-level callers handle FALSE correctly.

Tests

3 regression tests added to FieldFormatterTest:

  • test_typed_field_without_value_returns_false — options-page field object shape (with _name/_valid), no value key → FALSE
  • test_scalar_field_with_null_value_returns_falsetext and group with value => nullFALSE
  • test_repeater_and_flexible_with_missing_value_key_return_false — the Codex P2 negative case

Full suite: 848 tests, 1 527 assertions, green (4 pre-existing deprecations).

Downstream impact & rollout

  • Behaviour restored, not changed: projects written against ≤1.7 semantics ("unfilled option fields are absent") work again after upgrading. The only intentional difference vs 1.16.0 is that raw definition arrays can no longer reach templates.
  • mairateam is blocked on this: mairateam#37 bumps to 1.16.0 whose lock still carries this bug (author pages break with an unfilled options DB). Its vendor copy is interim-patched locally; after this merges + releases (suggest v1.16.1), mairateam bumps the lock to the released version and qa: composer hygiene + audit gates, modernise to PHPUnit 12 #37 becomes deployable.
  • Any downstream on 1.8–1.16.0 with unfilled options-page fields is exposed to the same class of "Component template not found" masking — worth a fleet-wide lock bump after release.

🤖 Generated with Claude Code

https://claude.ai/code/session_014N3Z17XG2gsXYRRqAjv4xP

@parisek parisek self-assigned this Jul 7, 2026
formatFields('option') on timber-kit >=1.8 surfaces options-page groups
via local-store walks; a field with no saved value arrives WITHOUT a
'value' key and fieldFormatter passed the raw ACF field-definition
array into templates - {{ x|typography }} then fatals on array input
(swallowed by the component fallback as 'template not found').

Missing 'value' key now returns FALSE (mapFields drops the key, isset
guards in templates work as pre-1.8); present-but-null value keeps the
documented repeater/flexible pass-through.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014N3Z17XG2gsXYRRqAjv4xP
@parisek parisek force-pushed the fix/field-formatter-valueless-fields branch from f23586f to 236fa27 Compare July 7, 2026 15:20
parisek and others added 2 commits July 7, 2026 17:26
…eview P2)

The null pass-through contract is only for explicit value => null (block
previews); a field object with no value key at all must read as empty
for every type. Negative tests added.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014N3Z17XG2gsXYRRqAjv4xP
@parisek parisek marked this pull request as ready for review July 7, 2026 15:38
@parisek parisek merged commit d23c9d8 into main Jul 7, 2026
6 checks passed
@parisek parisek deleted the fix/field-formatter-valueless-fields branch July 7, 2026 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant