Overview

For the first time, this config schema type now relies on type: sequence. For DX purposes (debugging, config diffing, etc.), it's important that the sequence keys are ordered A) consistently, B) sensibly.

IMHO the most sensible ordering would be:

  1. first by level in the tree, starting with the top of the tree
  2. within a level, in sibling order
  3. when there's multiple slots, by slot machine name order

That could be achieved by generating keys for each sequence item that represent the path to that component instance:

  • Conceptually: root-instance.slot_name.level1-instance.slot_name.level2-instance
  • Concretely: all those instances would be UUIDs — that'd be insanely long very quickly
  • So instead: don't use the UUID of a component instance, but the sibling index within each level, so that'd be 0.foo.1.bar.3 as the key for the (starting at the deepest level) 4th component instance in the bar slot of the second component instance in the foo slot of the first component instance in the root.

If we did that, then all we'd need to do is add orderby: key to config schema, and the config save/export functionality would automatically sort the tree exactly in that sensible ordering!

Before this MR, that didn't matter a great deal. But after this MR, it's trivially possible to run a DB query that either lists the entire tree, or a subtree, and manipulate the ordering with a simple ORDER BY. It's important that the DX for a developer (or site builder) inspecting big component trees in config is not painful.

That's not a blocker for this issue/MR though; but it should be a stable blocker. Let's create a follow-up issue for that.


https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...

This is that follow-up.

Proposed resolution

User interface changes

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

wim leers created an issue. See original summary.

wim leers’s picture

Issue summary: View changes
larowlan’s picture

We have to be wary here because order is important.

With content entity versions of these trees, there is a delta field that keeps track of the order.

Here there is not, and perhaps we should introduce it.

The delta keeps track of what order sibling components (those with the same parent/slot) appear in.

wim leers’s picture

Here there is not, and perhaps we should introduce it.

Here it is indeed the sequence order (which implicitly has "deltas" as keys: a PHP "list" array, so 0, 1 etc.) that ensures the correct ordering.

I'm contemplating whether this should be a beta blocker or not. It feels like it'd be safer to make this a beta blocker. Do you agree?

larowlan’s picture

Issue tags: +beta blocker

It feels like it should be simple enough - the main impacts here are on page regions and patterns. Add content templates if that ends up being in beta.

Tagging as such

nagwani’s picture

As beta blocker tag is added, I am removing stable blocker tag

nagwani’s picture

Issue tags: -stable blocker
larowlan’s picture

Assigned: Unassigned » larowlan

🏗️ picking this up

larowlan’s picture

Assigned: larowlan » Unassigned
Status: Active » Needs review

This should be ready for review 🤞

wim leers’s picture

Assigned: Unassigned » wim leers
wim leers’s picture

Assigned: wim leers » Unassigned
Status: Needs review » Reviewed & tested by the community

You made this look simple! 🤩

Only comment nits! Automated tests were crystal clear; no remarks. Also manually tested, just to make sure we didn't miss anything in the automated tests: works great 👍

wim leers’s picture

Title: Ensure predictable config export order of config-defined component trees » Ensure deterministic config export order of config-defined component trees

  • wim leers committed 7c299d95 on 0.x authored by larowlan
    Issue #3526127 by larowlan, wim leers: Ensure deterministic config...
wim leers’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.