fix(atomic): race condition in atomic-tab-manager attribute reading during initialization

[Copilot]

Fix Race Condition in atomic-tab-manager

Problem

Users intermittently encounter a SchemaValidationError when initializing atomic-tab-manager:

SchemaValidationError: The following properties are invalid: expression: value is required.

Root Cause

Race condition between parent and child Lit component initialization.

When atomic-tab-manager.initialize() executes synchronously (triggered by the @bindings() decorator's context callback), it reads child atomic-tab elements' properties:

expression: tabElement.expression,  // ← Can be undefined!
name: tabElement.name,              // ← Can also be undefined!
label: tabElement.label,            // ← Can also be undefined!

The issue is that Lit's @property decorator defines reactive properties asynchronously during component upgrade. When the race condition occurs:

1. HTML parser parses the DOM and attributes are set
2. Parent's initialize() is called synchronously
3. Child's Lit property accessors may not be configured yet
4. tabElement.expression, tabElement.name, and tabElement.label return undefined instead of the attribute values
5. Headless buildTab() validates with requiredEmptyAllowedString schema and throws

Solution

Use getAttribute() instead of Lit property accessors for all attributes, following the pattern from atomic-sort-dropdown.ts:

const name = tabElement.getAttribute('name') || '';
const expression = tabElement.getAttribute('expression') ?? '';
const label = tabElement.getAttribute('label') || '';

Why this works:

• HTML attributes are available immediately after DOM parsing
• getAttribute() is synchronous and doesn't depend on Lit's property initialization
• This pattern already exists in the codebase (see atomic-sort-dropdown.ts)
• Storing values in variables makes the code cleaner and more efficient (avoiding repeated calls)

Why fallback operators are necessary:

1. TypeScript compilation requirement: getAttribute() returns string | null, but the TabInfo interface requires string (non-nullable). The fallback operators are needed for the code to compile.

2. For name (uses || ''):

   • In practice, an empty string would be caught by the !name validation check (line 94) which throws an error before reaching this.tabs.push()
   • Additionally, atomic-tab's ValidatePropsController ensures name is required and non-empty when the component connects
   • The fallback ensures TypeScript type safety without affecting runtime behavior

3. For label (uses || ''):

   • In practice, an empty label would be caught by atomic-tab's ValidatePropsController which validates it's required and non-empty when the component connects
   • The fallback ensures TypeScript type safety without affecting runtime behavior

4. For expression (uses ?? ''):

   • Functionally necessary (not just for TypeScript)
   • When the attribute doesn't exist, getAttribute() returns null
   • The ?? '' converts null to an empty string which the schema validator accepts
   • Without it, buildTab() would throw because requiredEmptyAllowedString rejects null

Why NOT use ChildrenUpdateCompleteMixin (previously tried approach)

The mixin overrides getUpdateComplete() to wait for children's async update cycles. However:

• initialize() is called synchronously in the bindings callback
• No code awaits updateComplete before reading child properties
• The mixin's async behavior is never triggered during initialization

Why NOT use ?? '' on properties alone (nullish coalescing on property access, another previously tried approach)

If attributes are explicitly set in markup but properties aren't initialized yet:

• tabElement.expression ?? '' would return '' when expression is undefined
• The actual expression value would be silently ignored
• This would cause silent failures instead of fixing the issue

Testing

• Existing unit tests pass (10 tests)
• The fix follows established patterns in the codebase (atomic-sort-dropdown.ts uses the same approach)

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[Copilot]

Pull request overview

This PR fixes an intermittent race condition in atomic-tab-manager that causes a SchemaValidationError when initializing tabs. The issue occurs when the parent component reads child atomic-tab properties before Lit's async property descriptor setup completes, causing expression to return undefined instead of its default value ''. The fix adds nullish coalescing (??) as a defensive fallback, ensuring the expression is always defined regardless of initialization timing.

Key changes:

• Added ?? '' fallback when reading tabElement.expression in atomic-tab-manager.ts
• Added comprehensive unit test that explicitly simulates the race condition using Object.defineProperty to force undefined return values

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
packages/atomic/src/components/search/atomic-tab-manager/atomic-tab-manager.ts	Adds nullish coalescing operator to handle undefined expression property during race condition
packages/atomic/src/components/search/atomic-tab-manager/atomic-tab-manager.spec.ts	Adds test that forces expression property to return undefined and verifies empty string fallback

---

The code changes look excellent! The fix is minimal, correct, and follows established patterns in the codebase. The test properly simulates the race condition and validates the fix. This is a well-executed solution to a difficult-to-reproduce timing issue.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.