Align the property / child determination in Element class

Created on 6 February 2022, over 2 years ago
Updated 4 February 2023, over 1 year ago

Problem/Motivation

Follow-up from comment by @alexpott #3172166-34: Element::properties() produces notices if given an array with integer keys β†’ .

The Element::child() must be the inverse of Element::property() - so let's make this true in code. Element::children() should also follow the same logic.

Proposed resolution

For Element::child()

   public static function child($key) {
-    return !isset($key[0]) || $key[0] != '#';
+    return !static::property($key);
   }

For "Element::children() we have if (is_int($key) || $key === '' || $key[0] !== '#') { I guess we're inlining this here for performance - which makes sense (up to a point - this stuff is called thousands of times so often the measuring makes it seem way way slower than it actually is). But regardless - we can still make this more efficient by doing if (!(is_string($key) && $key[0] !== '#')). "

"Note the performance arguments for Element::child() don't really hold based on core usage."

Remaining tasks

Make changes.
Does this also require checking for performance for Element::children()?

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

πŸ“Œ Task
Status

Needs work

Version

10.1 ✨

Component
RenderΒ  β†’

Last updated about 15 hours ago

Created by

πŸ‡³πŸ‡±Netherlands ekes

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request β†’ as a guide.

    Seems there were failures in last patch.

    Even though this is a task will it require a test?

Production build 0.69.0 2024