From 002e05b175e384e4d89b71c1e41c3ed18fe943ab Mon Sep 17 00:00:00 2001 From: William Kelly Date: Fri, 5 Dec 2025 12:28:16 -0700 Subject: [PATCH 1/3] Perf/memory native crypto hashing (#2201) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf(memory): native crypto hashing + recursive object interning Optimize merkle hashing performance through two complementary approaches: ## 1. Native Crypto Hashing Switch from @noble/hashes (pure JS) to node:crypto (native OpenSSL with SHA-NI hardware acceleration) for merkle reference computation. - Use merkle-reference's Tree.createBuilder() API with custom hash fn - Leverage built-in WeakMap caching for sub-object reuse ## 2. Recursive Object Interning Add intern() function that deduplicates objects by JSON content, enabling merkle-reference's WeakMap cache to hit on shared nested content. - Integrated into Fact.assert() and Fact.unclaimed() automatically - Uses WeakRef + FinalizationRegistry for automatic garbage collection - Recursively interns nested objects for maximum cache hits ## Performance Improvements (16KB payloads) - Shared content across facts: ~2.5x faster (286µs → 71µs) - Repeated {the, of} patterns: ~62x faster (25µs → 0.4µs) - Overall set fact: ~786µs, get fact: ~58µs, retract: ~394µs ## Files Changed - reference.ts: Add native crypto + intern() function - fact.ts: Integrate interning into assert/unclaimed/normalizeFact - HASHING.md: Document optimization journey and benchmarks - test/memory_bench.ts: Add comprehensive interning benchmarks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * fix(memory): update bench task to use correct benchmark file The benchmark file was renamed from benchmark.ts to memory_bench.ts but the deno.json task wasn't updated to match. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * perf(memory): replace WeakRef with strong LRU cache in intern() The previous WeakRef-based intern cache had a fundamental flaw: GC would collect interned objects between refer() calls when no strong reference held them. This prevented merkle-reference's WeakMap from getting cache hits on repeated identical content. Changes: - Replace WeakRef with direct object storage in internCache Map - Add LRU eviction at 10,000 entries to bound memory - Add WeakSet for O(1) early return on already-interned objects - Remove FinalizationRegistry (no longer needed) The strong reference approach ensures interned objects stay alive long enough for refer() to benefit from merkle-reference's identity-based WeakMap cache. Benchmarks show ~2.5x speedup on shared content. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * perf(memory): add unclaimedRef() to cache unclaimed references Add a Map-based cache for unclaimed fact references. The common pattern of refer(unclaimed({the, of})) was being recomputed on every call, costing ~29µs each time. Changes: - Add unclaimedRefCache Map keyed by "${the}|${of}" - Add unclaimedRef() function that caches the full Reference - Update assert() and normalizeFact() to use unclaimedRef() Cache hits return in ~0.4µs vs ~29µs for a fresh refer() call, providing ~62x speedup for repeated {the, of} patterns. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * perf(memory): optimize refer() ordering and use cached references Multiple optimizations to reduce refer() overhead in the write path: 1. Use unclaimedRef() instead of refer(unclaimed(...)) in: - recall() for cause field - getFact() for cause field - toFact() for cause field - swap() for base reference - commit() for initial cause 2. Reorder refer() calls in swap() to maximize cache hits: - Compute fact hash BEFORE importing datum - When refer(assertion) traverses, it caches the payload hash - The subsequent refer(datum) in importDatum() hits cache (~300ns) - Previously: datum first (missed cache opportunity) - This saves ~25% on refer() time (~50-100µs per operation) 3. Intern transaction before creating commit: - Ensures all nested objects share identity - refer(assertion) caches sub-object hashes - refer(commit) hits those caches (~26% faster commits) Overall setFact improvement: ~700µs → ~664µs (5-10% faster) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * docs(memory): comprehensive hashing performance documentation Expanded HASHING.md with detailed findings from performance investigation: - Executive summary: 90% of refer() time is structural overhead, not hashing - How merkle-reference works internally (toTree → digest → fold) - Time breakdown showing where ~190µs actually goes for 16KB payload - Why nested transaction schema (4 levels) is expensive (~77-133µs overhead) - setFact breakdown: ~664µs total, 71% in refer() calls - Key findings: native crypto, WeakMap caching, call order, intern benefits - What didn't work: small object cache patterns - Current implementation with code examples - Optimization opportunities ranked by impact (immediate → breaking) - Realistic expectations table with potential improvements - Architecture notes on why content-addressing requires this overhead This document serves as a reference for future optimization work and explains why we're approaching the fundamental floor for content-addressed storage. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * fix(memory): use conditional crypto for browser compatibility - Browser: use merkle-reference's default refer() (uses @noble/hashes internally) - Server: upgrade to node:crypto TreeBuilder for ~1.5-2x speedup - Dynamic import prevents bundlers from resolving node:crypto in browser builds - Fixes CORS/module resolution error when shell tries to import node:crypto 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * perf(memory): add SQLite pragmas for WAL mode and performance - journal_mode=WAL: Better concurrency, faster writes - synchronous=NORMAL: Safe for WAL, improved write performance - busy_timeout=5000: Wait on locks instead of failing - page_size=32768: 32KB pages for new databases - cache_size=-64000: ~64MB in-memory page cache - temp_store=MEMORY: Keep temp tables in RAM - mmap_size=268435456: 256MB memory-mapped I/O - foreign_keys=ON: Enforce referential integrity Benchmarks show ~3x faster single writes/updates and ~1.5x faster reads on file-based databases. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * test(memory): add file-based benchmarks for pragma testing Add file-based benchmark group to measure real WAL mode and pragma impact on disk I/O. Memory-based benchmarks don't exercise WAL/mmap. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * fix(memory): address PR review comments - Fix LRU cache recency bug in intern(): now properly moves accessed entries to end of Map via delete+re-insert - Replace custom isBrowser detection with isDeno() from @commontools/utils/env - Fix type error in unclaimedRef by using Ref.View to match what refer() returns 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * fix(memory): update benchmark to use unclaimedRef The benchmark was testing the old refer() pattern directly, but the PR changed the caching strategy to use unclaimedRef() for unclaimed facts. Update the benchmark to test the actual production code path. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * fix(deps): restore deno.lock from main to fix OpenTelemetry types The lock file had inadvertently downgraded @opentelemetry/sdk-trace-base from 1.30.1 to 1.19.0, which doesn't support the spanProcessors constructor option used in otel.ts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * fix(memory): remove unused import and fix formatting - Remove unused `unclaimed` import from space.ts - Apply deno fmt to HASHING.md and space.ts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --------- Co-authored-by: Claude --- packages/memory/HASHING.md | 564 +++++++++++++++++++++++++++ packages/memory/deno.json | 2 +- packages/memory/fact.ts | 51 ++- packages/memory/reference.ts | 145 +++++-- packages/memory/space.ts | 72 +++- packages/memory/test/memory_bench.ts | 559 +++++++++++++++++++++++++- 6 files changed, 1333 insertions(+), 60 deletions(-) create mode 100644 packages/memory/HASHING.md diff --git a/packages/memory/HASHING.md b/packages/memory/HASHING.md new file mode 100644 index 0000000000..e368dcc6d7 --- /dev/null +++ b/packages/memory/HASHING.md @@ -0,0 +1,564 @@ +# Merkle Hashing Performance in Memory Package + +This document describes the performance optimizations made to merkle hashing in +the memory package, what we learned, and decisions made along the way. + +## Executive Summary + +For a typical `setFact` operation with a 16KB payload: + +- Total time: ~664µs +- ~71% spent in `refer()` calls (~470µs) +- Of that, **only ~10% is actual SHA-256 hashing** +- **90% of refer() time is structural overhead** (sorting, traversal, + allocations) + +## Background + +The memory package uses `merkle-reference` to compute content-addressable hashes +for facts. Every `set`, `update`, and `retract` operation requires computing +merkle hashes for assertions, transactions, and payloads. + +Initial profiling showed that hashing was a significant bottleneck, with +`refer()` calls dominating transaction time despite SQLite being very fast +(~20µs for 16KB inserts). + +## How merkle-reference Works Internally + +The library computes content-addressed hashes via: + +``` +refer(source) → toTree(source) → digest(tree) → Reference +``` + +### Algorithm Steps + +1. **toTree()**: Recursively converts JS objects to a tree structure + - WeakMap lookup per node (cache check) + - Type dispatch (object, array, string, number, etc.) + - For objects: UTF-8 encode all keys, sort alphabetically, encode each value + +2. **digest()**: Computes hash of tree + - WeakMap lookup per node (cache check) + - Leaf nodes: hash directly + - Branch nodes: collect child digests, fold into binary tree + +3. **fold()**: Combines digests via binary tree reduction + - Pairs of digests hashed together + - Repeated until single root hash + +### Object Encoding (The Expensive Part) + +```javascript +// From merkle-reference map.js +for (const [name, value] of entries) { + const key = builder.toTree(name); + const order = typeof name === "string" + ? String.toUTF8(name) // UTF-8 encode for sorting + : builder.digest(key); + + attributes.push({ order, key, value: builder.toTree(value) }); +} + +// EXPENSIVE: Sort all attributes by byte comparison +return attributes.sort((left, right) => compare(left.order, right.order)); +``` + +**Key insight**: Every object requires UTF-8 encoding all keys + sorting them. +This is required for deterministic merkle tree construction across different +systems, but it's expensive. + +## Time Breakdown: Where Does refer() Time Go? + +For a 16KB payload taking ~190µs: + +| Operation | Time | % | Notes | +| -------------------------- | ------- | --- | ------------------------------ | +| Actual SHA-256 hashing | 10-20µs | 10% | Native crypto on ~16KB | +| Key sorting (objects) | 40-50µs | 25% | UTF-8 encode + byte comparison | +| Object traversal + WeakMap | 30-40µs | 20% | ~2µs per node × ~15-20 nodes | +| UTF-8 encoding overhead | 20-30µs | 15% | TextEncoder on string keys | +| Tree node allocations | 30-40µs | 20% | Arrays for branches | +| Fold operation | 20-30µs | 10% | Binary tree reduction | + +**Key finding**: Only ~10% of time is actual hashing. The other ~90% is +structural overhead required for deterministic merkle tree construction. + +## Why Nested Transaction Schema is Expensive + +### Current Changes Structure (4 levels deep) + +```typescript +{ + changes: { + [of]: { // Level 1: entity URI + [the]: { // Level 2: MIME type + [cause]: { // Level 3: cause reference + is: { /* payload */ } // Level 4: actual data + } + } + } + } +} +``` + +### Full Transaction Tree (~7 nested objects) + +``` +Transaction (8 keys) → args (1 key) → changes (1 key) → entity (1 key) + → mime (1 key) → cause (1 key) → is (payload with ~5 keys) +``` + +### Cost Per Object Level + +Each nested object requires: + +- WeakMap lookup (nodes): ~2µs +- WeakMap lookup (digests): ~2µs +- Sort operation: ~5-10µs (small), ~20-40µs (many keys) +- Array allocations: ~2-5µs + +**Total per level: ~11-19µs** + +For 7 nested objects: **~77-133µs just for structure overhead** + +## setFact Operation Breakdown (~664µs) + +| Component | Time | % | +| --------------------- | -------- | ----- | +| refer(user assertion) | ~370µs | 56% | +| refer(commit) | ~100µs | 15% | +| intern(transaction) | ~60µs | 9% | +| SQLite (3-4 INSERTs) | ~60-80µs | 9-12% | +| JSON.stringify | ~8µs | 1% | +| Other overhead | ~30-66µs | 5-10% | + +### Why Two swap() Calls Per Transaction? + +Each `setFact` triggers two `swap()` calls: + +1. **User fact**: The actual assertion (~350-550µs) +2. **Commit record**: Audit trail containing full transaction (~100-150µs) + +The commit record embeds the entire transaction for audit/sync purposes. + +## Key Findings + +### 1. SHA-256 Implementation Matters + +`merkle-reference` uses `@noble/hashes` for SHA-256, which is a pure JavaScript +implementation. On modern CPUs with SHA-NI (hardware SHA acceleration), +`node:crypto` is significantly faster for raw hashing operations. + +The exact speedup varies by payload size, but native crypto typically provides +2-10x improvement on the hash computation itself. The gap widens with payload +size because node:crypto uses native OpenSSL with hardware SHA-NI instructions. + +Note: The end-to-end `refer()` time includes more than just hashing (sorting, +tree traversal, object allocation), so the overall speedup is smaller than the +raw hash speedup. + +**Environment-Specific Behavior**: The memory package uses conditional crypto: + +- **Browser environments** (shell): Uses `@noble/hashes` (pure JS, works + everywhere) +- **Server environments** (toolshed, Node.js, Deno): Uses `node:crypto` for + hardware-accelerated performance + +This is detected at module load time via +`globalThis.document`/`globalThis.window` and uses dynamic import to avoid +bundler issues with `node:crypto` in browsers. + +### 2. merkle-reference Caches Sub-objects by Identity + +`merkle-reference` uses a `WeakMap` internally to cache computed tree nodes and +digests by object identity. When computing a merkle hash: + +1. It recursively traverses the object tree +2. For each sub-object, it checks its WeakMap cache +3. If the same object instance was seen before, it reuses the cached digest +4. Only new/unseen objects require hash computation + +This means: + +- If you pass the **same object instance** multiple times, subsequent calls are + very fast (WeakMap lookup ~300ns) +- If you pass a **new object with identical content**, it must recompute the + full hash (different object identity = cache miss) + +This is crucial for our use case: assertions contain a payload (`is` field). If +the payload object is reused across assertions, merkle-reference can skip +re-hashing it entirely: + +```typescript +const payload = { content: "..." }; // 16KB + +// First assertion - full hash computation for payload + assertion wrapper +refer({ the: "app/json", of: "doc1", is: payload }); // ~250µs + +// Second assertion with SAME payload object - only hash the new wrapper +// The payload's digest is retrieved from WeakMap cache +refer({ the: "app/json", of: "doc2", is: payload }); // ~70µs (3.5x faster) +``` + +**Cache size:** The cache is automatically bounded by the garbage collector +because it uses `WeakMap`. When a source object is no longer referenced anywhere +in your application, its cache entry is automatically collected. + +### 3. Order of refer() Calls Matters + +In `swap()`, we compute hashes in a specific order to maximize cache hits: + +```typescript +// IMPORTANT: Compute fact hash BEFORE importing datum. When refer() traverses +// the assertion/retraction, it computes and caches the hash of all sub-objects +// including the datum (payload). By hashing the fact first, the subsequent +// refer(datum) call in importDatum() becomes a ~300ns cache hit instead of a +// ~50-100µs full hash computation. +const fact = refer(source.assert).toString(); // Caches payload hash +const datumRef = importDatum(session, is); // Cache hit on payload! +``` + +### 4. intern(transaction) is Beneficial + +The `intern(transaction)` call (~18µs) provides ~26% speedup on `refer(commit)`: + +| Scenario | refer(commit) | Total | +| -------------- | ------------- | ----- | +| Without intern | 116µs | 146µs | +| With intern | 58µs | 108µs | + +**Mechanism**: Interning ensures all nested objects share identity. When +`refer(assertion)` runs first, it caches all sub-object hashes. When +`refer(commit)` runs, it hits those caches because the assertion objects inside +the commit are the exact same instances. + +### 5. Tree Builder API + +`merkle-reference` exposes `Tree.createBuilder(hashFn)` which allows overriding +the hash function while preserving the merkle tree structure and caching +behavior. + +```typescript +import { Tree } from "merkle-reference"; +import { createHash } from "node:crypto"; + +const nodeSha256 = (payload: Uint8Array): Uint8Array => { + return createHash("sha256").update(payload).digest(); +}; + +const treeBuilder = Tree.createBuilder(nodeSha256); +treeBuilder.refer(source); // Uses node:crypto, same hash output +``` + +**Important:** Hashes are identical regardless of which SHA-256 implementation +is used. The tree structure and encoding are the same; only the underlying hash +function differs. + +## What Didn't Work + +### Small Object Cache for `{the, of}` Patterns + +We tried caching `{the, of}` patterns (unclaimed references) using a +string-keyed Map: + +```typescript +// REMOVED - actually hurt performance +const unclaimedCache = new Map(); +if (isUnclaimedPattern(source)) { + const key = source.the + "\0" + source.of; + // ...cache lookup... +} +``` + +This added ~20µs overhead per call due to: + +- `Object.keys()` check to detect the pattern +- String concatenation for cache key +- Map lookup + +merkle-reference's internal WeakMap is faster for repeated access to the same +object, and for unique objects there's no cache benefit anyway. + +**However**: `unclaimedRef()` with a simple Map cache DOES work well because it +caches the final Reference, not intermediate objects. This saves the entire +`refer()` call (~29µs) for repeated `{the, of}` combinations. + +## Current Implementation + +### 1. Conditional crypto hashing (browser vs server) + +Use merkle-reference's default `refer()` in browsers (which uses `@noble/hashes` +internally), upgrade to a custom TreeBuilder with `node:crypto` in server +environments for hardware acceleration: + +```typescript +import * as Reference from "merkle-reference"; + +// Default to merkle-reference's built-in refer (uses @noble/hashes) +let referImpl: (source: T) => Reference.View = Reference.refer; + +// In server environments, upgrade to node:crypto for better performance +const isBrowser = typeof globalThis.document !== "undefined" || + typeof globalThis.window !== "undefined"; + +if (!isBrowser) { + try { + // Dynamic import avoids bundler resolution in browsers + const nodeCrypto = await import("node:crypto"); + const nodeSha256 = (payload: Uint8Array): Uint8Array => { + return nodeCrypto.createHash("sha256").update(payload).digest(); + }; + const treeBuilder = Reference.Tree.createBuilder(nodeSha256); + referImpl = (source: T): Reference.View => { + return treeBuilder.refer(source) as unknown as Reference.View; + }; + } catch { + // node:crypto not available, use merkle-reference's default + } +} + +export const refer = (source: T): Reference.View => { + return referImpl(source); +}; +``` + +**Key design points:** + +- Browser: Uses `Reference.refer()` directly (merkle-reference uses + @noble/hashes) +- Server: Creates custom TreeBuilder with `node:crypto` for ~1.5-2x speedup +- Dynamic import (`await import()`) prevents bundlers from resolving + `node:crypto` +- Environment detection via `globalThis.document`/`globalThis.window` + +### 2. Recursive object interning + +To enable cache hits on identical content (not just identical object instances), +we intern objects recursively with a strong LRU cache: + +```typescript +const INTERN_CACHE_MAX_SIZE = 10000; +const internCache = new Map(); +const internedObjects = new WeakSet(); + +export const intern = (source: T): T => { + if (source === null || typeof source !== "object") return source; + if (internedObjects.has(source)) return source; // Fast path + + // Recursively intern nested objects first + const internedObj = Array.isArray(source) + ? source.map((item) => intern(item)) + : Object.fromEntries( + Object.entries(source).map(([k, v]) => [k, intern(v)]), + ); + + const key = JSON.stringify(internedObj); + const cached = internCache.get(key); + if (cached) return cached as T; + + // LRU eviction + if (internCache.size >= INTERN_CACHE_MAX_SIZE) { + const firstKey = internCache.keys().next().value; + if (firstKey) internCache.delete(firstKey); + } + + internCache.set(key, internedObj); + internedObjects.add(internedObj); + return internedObj as T; +}; +``` + +### 3. unclaimedRef() caching + +For the common `{the, of}` pattern (unclaimed facts), we cache the entire +Reference to avoid repeated `refer()` calls: + +```typescript +const unclaimedRefCache = new Map>(); + +export const unclaimedRef = ( + { the, of }: { the: MIME; of: URI }, +): Reference => { + const key = `${the}|${of}`; + let ref = unclaimedRefCache.get(key); + if (!ref) { + ref = refer(unclaimed({ the, of })); + unclaimedRefCache.set(key, ref); + } + return ref; +}; +``` + +## Optimization Opportunities + +### Immediate Wins (No Breaking Changes) + +#### 1. Use Shared Empty Arrays (~5-10µs savings) + +```typescript +// Before +prf: []; // New array each time + +// After +const EMPTY_ARRAY = Object.freeze([]); +prf: EMPTY_ARRAY; // Reuse, enables WeakMap cache hits +``` + +### Medium-Term (Requires Library Support) + +#### 2. Pre-sort Transaction Keys (~20-30µs potential) + +If merkle-reference detected pre-sorted keys, we could skip sorting: + +```typescript +// Keys in alphabetical order +return { + args: { changes }, // 'a' comes first + cmd: "/memory/transact", + exp: iat + ttl, + iat, + iss: issuer, + prf: EMPTY_ARRAY, + sub: subject, +}; +``` + +**Note**: Currently merkle-reference doesn't detect this, so no benefit yet. + +#### 3. Library Optimizations (Upstream Contributions) + +- Skip sorting for single-key objects +- Cache UTF-8 encoded keys for common strings +- Detect pre-sorted keys + +### Long-Term (Breaking Changes) + +#### 4. Flatten Changes Structure (~50-70µs savings, 26-37% faster) + +**Current** (4 levels): + +```typescript +{ [of]: { [the]: { [cause]: { is } } } } +``` + +**Proposed** (flat array): + +```typescript +[ { of, the, cause, is }, { of, the, cause, is }, ... ] +``` + +**Benefits**: + +- Eliminates 2 object traversals (~40µs) +- Arrays don't require key sorting (~20-30µs) +- Simpler tree = fewer allocations (~10µs) + +**Tradeoffs**: + +- Breaking change to transaction format +- Larger serialized size (repeated keys) +- Less convenient for lookups + +#### 5. Skip Commit Records for Single-Fact Transactions (~100-150µs savings) + +Currently every transaction writes a commit record for audit trail. For +single-fact transactions, this could be optional. + +**Tradeoff**: Loses transaction-level audit trail. + +## Realistic Expectations + +| Optimization Level | Expected Time | Improvement | +| ---------------------- | ------------- | ----------- | +| Current | 664µs | baseline | +| With immediate wins | ~640µs | 4% | +| With all non-breaking | ~600µs | 10% | +| With flattened Changes | ~540µs | 19% | +| With skip commit | ~440µs | 34% | + +**Fundamental floor**: Object traversal + deterministic ordering will always +consume ~100-120µs for nested structures. This is inherent to +content-addressing. + +## Performance Results + +### Core Operations (16KB payloads) + +| Operation | Time | Throughput | +| ----------------- | ------ | ---------- | +| get fact (single) | ~65µs | 15,000/s | +| set fact (single) | ~664µs | 1,500/s | +| update fact | ~756µs | 1,320/s | +| retract fact | ~436µs | 2,300/s | + +### Component Breakdown + +| Component | Time | Notes | +| ------------------------ | ------- | -------------------------- | +| Raw SQLite INSERT | 20-35µs | Hardware floor | +| JSON.stringify 16KB | ~8µs | | +| refer() on 16KB | ~190µs | Payload only | +| refer() on assertion | ~470µs | Includes 16KB payload | +| refer() small object | ~34µs | {the, of} pattern | +| unclaimedRef() cache hit | ~0.4µs | Returns cached Reference | +| intern() cache hit | <1µs | Returns canonical instance | + +## Benchmarks Reference + +Run benchmarks with: + +```bash +deno task bench +``` + +Key isolation benchmarks to watch: + +- `refer() on 16KB payload (isolation)`: ~190µs +- `refer() on assertion (16KB is + metadata)`: ~470µs +- `memoized: 3x refer() same payload (cache hits)`: ~24µs +- `refer() small {the, of} - with intern (cache hit)`: ~0.4µs + +## Architecture Notes + +### Why Content-Addressing? + +merkle-reference provides: + +- Deduplication (same content = same hash) +- Integrity verification +- Distributed sync compatibility +- Deterministic references + +**Cannot be eliminated** without breaking the architecture. + +### Deterministic Ordering Requirement + +For merkle trees to produce consistent hashes across different systems, object +keys must be sorted deterministically. This is why: + +- Every object incurs sorting cost +- UTF-8 encoding needed for byte-comparison +- This overhead is fundamental to the approach + +## Current Optimizations Applied + +1. **Conditional crypto** (node:crypto in server, @noble/hashes in browser): + ~1.5-2x speedup on hashing in server environments, while maintaining browser + compatibility +2. **Recursive object interning**: ~2.5x on shared content +3. **Prepared statement caching**: ~2x on queries +4. **Batch label lookups**: Eliminated N queries +5. **Fact hash ordering**: Payload hash reused from assertion traversal +6. **Stored fact hashes**: Avoid recomputing in conflict detection +7. **unclaimedRef() caching**: ~62x faster for repeated {the, of} patterns +8. **intern(transaction)**: ~26% faster commits via cache hits + +## Files Reference + +- `reference.ts`: TreeBuilder with conditional crypto (noble/node:crypto), + intern() function +- `fact.ts`: Fact.assert(), unclaimedRef() caching +- `space.ts`: swap(), commit(), transact() - core write path +- `transaction.ts`: Transaction structure definition +- `changes.ts`: Changes structure (candidate for flattening) diff --git a/packages/memory/deno.json b/packages/memory/deno.json index fb9fc759a5..00f71e913b 100644 --- a/packages/memory/deno.json +++ b/packages/memory/deno.json @@ -20,7 +20,7 @@ }, "bench": { "description": "Run benchmarks for fact operations", - "command": "deno bench --allow-read --allow-write --allow-net --allow-ffi --allow-env --no-check test/benchmark.ts" + "command": "deno bench --allow-read --allow-write --allow-net --allow-ffi --allow-env --no-check test/memory_bench.ts" } }, "test": { diff --git a/packages/memory/fact.ts b/packages/memory/fact.ts index 789db97d88..81a80e3858 100644 --- a/packages/memory/fact.ts +++ b/packages/memory/fact.ts @@ -11,17 +11,45 @@ import { State, Unclaimed, } from "./interface.ts"; -import { fromJSON, fromString, is as isReference, refer } from "./reference.ts"; +import * as Ref from "./reference.ts"; +import { + fromJSON, + fromString, + intern, + is as isReference, + refer, +} from "./reference.ts"; /** * Creates an unclaimed fact. + * Interned so repeated {the, of} patterns share identity for cache hits. */ export const unclaimed = ( { the, of }: { the: MIME; of: URI }, -): Unclaimed => ({ - the, - of, -}); +): Unclaimed => intern({ the, of }); + +/** + * Cache for unclaimed references. + * Caches the refer() result so repeated calls with same {the, of} are O(1). + * This saves ~29µs per call (refer cost on small objects). + */ +const unclaimedRefCache = new Map>(); + +/** + * Returns a cached merkle reference to an unclaimed fact. + * Use this instead of `refer(unclaimed({the, of}))` for better performance. + */ +export const unclaimedRef = ( + { the, of }: { the: MIME; of: URI }, +): Ref.View => { + const key = `${the}|${of}`; + let ref = unclaimedRefCache.get(key); + if (!ref) { + ref = refer(unclaimed({ the, of })); + unclaimedRefCache.set(key, ref); + } + return ref; +}; export const assert = ({ the, @@ -37,16 +65,17 @@ export const assert = ({ ({ the, of, - is, + // Intern the payload so identical content shares identity for cache hits + is: intern(is), cause: isReference(cause) ? cause : cause == null - ? refer(unclaimed({ the, of })) + ? unclaimedRef({ the, of }) : refer({ the: cause.the, of: cause.of, cause: cause.cause, - ...(cause.is ? { is: cause.is } : undefined), + ...(cause.is ? { is: intern(cause.is) } : undefined), }), }) as Assertion; @@ -146,20 +175,20 @@ export function normalizeFact< const newCause = isReference(arg.cause) ? arg.cause : arg.cause == null - ? refer(unclaimed({ the: arg.the, of: arg.of })) + ? unclaimedRef({ the: arg.the, of: arg.of }) : "/" in arg.cause ? fromJSON(arg.cause as unknown as { "/": string }) : refer({ the: arg.cause.the, of: arg.cause.of, cause: arg.cause.cause, - ...(arg.cause.is ? { is: arg.cause.is } : undefined), + ...(arg.cause.is ? { is: intern(arg.cause.is) } : undefined), }); if (arg.is !== undefined) { return ({ the: arg.the, of: arg.of, - is: arg.is, + is: intern(arg.is), cause: newCause, }) as Assertion; } else { diff --git a/packages/memory/reference.ts b/packages/memory/reference.ts index 901197399d..1b0f619de1 100644 --- a/packages/memory/reference.ts +++ b/packages/memory/reference.ts @@ -1,4 +1,5 @@ import * as Reference from "merkle-reference"; +import { isDeno } from "@commontools/utils/env"; export * from "merkle-reference"; // Don't know why deno does not seem to see there is a `fromString` so we just @@ -8,41 +9,133 @@ export const fromString = Reference.fromString as ( ) => Reference.View; /** - * Bounded LRU cache for memoizing refer() results. - * refer() is a pure function (same input → same output), so caching is safe. - * We use JSON.stringify as the cache key since it's ~25x faster than refer(). + * Refer function - uses merkle-reference's default in browsers (@noble/hashes), + * upgrades to node:crypto in server environments for ~1.5-2x speedup. + * + * Browser environments use merkle-reference's default (pure JS, works everywhere). + * Server environments (Node.js, Deno) use node:crypto when available. */ -const CACHE_MAX_SIZE = 1000; -const referCache = new Map(); +let referImpl: (source: T) => Reference.View = Reference.refer; + +// In Deno, try to use node:crypto for better performance +if (isDeno()) { + try { + // Dynamic import to avoid bundler resolution in browsers + const nodeCrypto = await import("node:crypto"); + const nodeSha256 = (payload: Uint8Array): Uint8Array => { + return nodeCrypto.createHash("sha256").update(payload).digest(); + }; + const treeBuilder = Reference.Tree.createBuilder(nodeSha256); + referImpl = (source: T): Reference.View => { + return treeBuilder.refer(source) as unknown as Reference.View; + }; + } catch { + // node:crypto not available, use merkle-reference's default + } +} /** - * Memoized version of refer() that caches results. - * Provides significant speedup for repeated references to the same objects, - * which is common in transaction processing where the same payload is - * referenced multiple times (datum, assertion, commit log). + * Object interning cache: maps JSON content to a canonical object instance. + * Uses strong references with LRU eviction to ensure cache hits. + * + * Previously used WeakRef, but this caused cache misses because GC would + * collect interned objects between calls when no strong reference held them. + * This prevented merkle-reference's WeakMap from getting cache hits. + * + * With strong references + LRU eviction, interned objects stay alive long + * enough for refer() to benefit from merkle-reference's identity-based cache. */ -export const refer = (source: T): Reference.View => { - const key = JSON.stringify(source); +const INTERN_CACHE_MAX_SIZE = 10000; +const internCache = new Map(); - let ref = referCache.get(key); - if (ref !== undefined) { - // Move to end (most recently used) by re-inserting - referCache.delete(key); - referCache.set(key, ref); - return ref as Reference.View; +/** + * WeakSet to track objects that are already interned (canonical instances). + * This allows O(1) early return for already-interned objects. + */ +const internedObjects = new WeakSet(); + +/** + * Recursively intern an object and all its nested objects. + * Returns a new object where all sub-objects are canonical instances, + * enabling merkle-reference's WeakMap cache to hit on shared sub-content. + * + * Example: + * const obj1 = intern({ id: "uuid-1", content: { large: "data" } }); + * const obj2 = intern({ id: "uuid-2", content: { large: "data" } }); + * // obj1.content === obj2.content (same object instance) + * // refer(obj1) then refer(obj2) will cache-hit on content + */ +export const intern = (source: T): T => { + // Only intern objects (not primitives) + if (source === null || typeof source !== "object") { + return source; + } + + // Fast path: if this object is already interned, return it immediately + if (internedObjects.has(source)) { + return source; } - // Compute new reference - ref = Reference.refer(source); + // Handle arrays + if (Array.isArray(source)) { + const internedArray = source.map((item) => intern(item)); + const key = JSON.stringify(internedArray); + const cached = internCache.get(key); + + if (cached !== undefined) { + // Move to end (most recently used) by re-inserting + internCache.delete(key); + internCache.set(key, cached); + return cached as T; + } - // Evict oldest entry if at capacity - if (referCache.size >= CACHE_MAX_SIZE) { - const oldest = referCache.keys().next().value; - if (oldest !== undefined) { - referCache.delete(oldest); + // Evict oldest entry if cache is full + if (internCache.size >= INTERN_CACHE_MAX_SIZE) { + const oldest = internCache.keys().next().value; + if (oldest !== undefined) internCache.delete(oldest); } + internCache.set(key, internedArray); + internedObjects.add(internedArray); + return internedArray as T; } - referCache.set(key, ref); - return ref as Reference.View; + // Handle plain objects: recursively intern all values first + const internedObj: Record = {}; + for (const [k, v] of Object.entries(source)) { + internedObj[k] = intern(v); + } + + const key = JSON.stringify(internedObj); + const cached = internCache.get(key); + + if (cached !== undefined) { + // Move to end (most recently used) by re-inserting + internCache.delete(key); + internCache.set(key, cached); + return cached as T; + } + + // Evict oldest entry if cache is full + if (internCache.size >= INTERN_CACHE_MAX_SIZE) { + const oldest = internCache.keys().next().value; + if (oldest !== undefined) internCache.delete(oldest); + } + // Store this object as the canonical instance + internCache.set(key, internedObj); + internedObjects.add(internedObj); + + return internedObj as T; +}; + +/** + * Compute a merkle reference for the given source. + * + * In server environments, uses node:crypto SHA-256 (with hardware acceleration) + * for ~1.5-2x speedup. In browsers, uses merkle-reference's default (@noble/hashes). + * + * merkle-reference's internal WeakMap caches sub-objects by identity, so passing + * the same payload object to multiple assertions will benefit from caching. + */ +export const refer = (source: T): Reference.View => { + return referImpl(source); }; diff --git a/packages/memory/space.ts b/packages/memory/space.ts index 5cd9c46c88..e44133478d 100644 --- a/packages/memory/space.ts +++ b/packages/memory/space.ts @@ -6,8 +6,8 @@ import { } from "@db/sqlite"; import { COMMIT_LOG_TYPE, create as createCommit } from "./commit.ts"; -import { unclaimed } from "./fact.ts"; -import { fromString, refer } from "./reference.ts"; +import { unclaimedRef } from "./fact.ts"; +import { fromString, intern, refer } from "./reference.ts"; import { addMemoryAttributes, traceAsync, traceSync } from "./telemetry.ts"; import type { Assert, @@ -135,6 +135,22 @@ JOIN COMMIT; `; +// Pragmas applied to every database connection +const PRAGMAS = ` + PRAGMA journal_mode=WAL; + PRAGMA synchronous=NORMAL; + PRAGMA busy_timeout=5000; + PRAGMA cache_size=-64000; + PRAGMA temp_store=MEMORY; + PRAGMA mmap_size=268435456; + PRAGMA foreign_keys=ON; +`; + +// Must be set before database has any content (new DBs only) +const NEW_DB_PRAGMAS = ` + PRAGMA page_size=32768; +`; + const IMPORT_DATUM = `INSERT OR IGNORE INTO datum (this, source) VALUES (:this, :source);`; @@ -411,6 +427,7 @@ export const connect = async ({ database = await new Database(address ?? ":memory:", { create: false, }); + database.exec(PRAGMAS); database.exec(PREPARE); const session = new Space(subject as Subject, database); return { ok: session }; @@ -446,6 +463,8 @@ export const open = async ({ database = await new Database(address ?? ":memory:", { create: true, }); + database.exec(NEW_DB_PRAGMAS); + database.exec(PRAGMAS); database.exec(PREPARE); const session = new Space(subject as Subject, database); return { ok: session }; @@ -505,7 +524,7 @@ const recall = ( of, cause: row.cause ? (fromString(row.cause) as Reference) - : refer(unclaimed({ the, of })), + : unclaimedRef({ the, of }), since: row.since, fact: row.fact, // Include stored hash to avoid recomputing with refer() }; @@ -585,7 +604,7 @@ const getFact = ( of: row.of as URI, cause: row.cause ? (fromString(row.cause) as Reference) - : refer(unclaimed(row as FactAddress)), + : unclaimedRef(row as FactAddress), since: row.since, }; if (row.is) { @@ -646,7 +665,7 @@ const toFact = function (row: StateRow): SelectedFact { of: row.of as URI, cause: row.cause ? row.cause as CauseString - : refer(unclaimed(row as FactAddress)).toString() as CauseString, + : unclaimedRef(row as FactAddress).toString() as CauseString, is: row.is ? JSON.parse(row.is) as JSONValue : undefined, since: row.since, }; @@ -760,27 +779,31 @@ const swap = ( ? [source.retract, source.retract.cause] : [source.claim, source.claim.fact]; const cause = expect.toString(); - const base = refer(unclaimed({ the, of })).toString(); + const base = unclaimedRef({ the, of }).toString(); const expected = cause === base ? null : (expect as Reference); - // IMPORTANT: Import datum BEFORE computing fact reference. The fact hash - // includes the datum as a sub-object, and merkle-reference caches sub-objects - // by identity during traversal. By hashing the datum first, we ensure the - // subsequent refer(assertion) call hits the cache for the payload (~2-4x faster). - let datumRef: string | undefined; - if (source.assert || source.retract) { - datumRef = importDatum(session, is); - } - // Derive the merkle reference to the fact that memory will have after // successful update. If we have an assertion or retraction we derive fact // from it, but if it is a confirmation `cause` is the fact itself. + // + // IMPORTANT: Compute fact hash BEFORE importing datum. When refer() traverses + // the assertion/retraction, it computes and caches the hash of all sub-objects + // including the datum (payload). By hashing the fact first, the subsequent + // refer(datum) call in importDatum() becomes a ~300ns cache hit instead of a + // ~50-100µs full hash computation. This saves ~25% on refer() time. const fact = source.assert ? refer(source.assert).toString() : source.retract ? refer(source.retract).toString() : source.claim.fact.toString(); + // Import datum AFTER computing fact reference - the datum hash is now cached + // from the fact traversal above, making this a fast cache hit. + let datumRef: string | undefined; + if (source.assert || source.retract) { + datumRef = importDatum(session, is); + } + // If this is an assertion we need to insert fact referencing the datum. let imported = 0; if (source.assert || source.retract) { @@ -880,11 +903,22 @@ const commit = ( (JSON.parse(row.is as string) as CommitData).since + 1, fromString(row.fact) as Reference, ] - : [0, refer(unclaimed({ the, of }))]; - - const commit = createCommit({ space: of, since, transaction, cause }); + : [0, unclaimedRef({ the, of })]; + + // Intern the transaction first so that: + // 1. createCommit() will reuse this exact transaction object (via WeakSet fast path) + // 2. iterateTransaction() uses the same objects that are in commit.is.transaction + // 3. When swap() hashes payloads, those same objects are in commit.is.transaction + // 4. refer(commit) can cache-hit on all sub-objects + const internedTransaction = intern(transaction); + const commit = createCommit({ + space: of, + since, + transaction: internedTransaction, + cause, + }); - for (const fact of iterateTransaction(transaction)) { + for (const fact of iterateTransaction(internedTransaction)) { swap(session, fact, commit.is); } diff --git a/packages/memory/test/memory_bench.ts b/packages/memory/test/memory_bench.ts index d162519e39..ea5da5a9c1 100644 --- a/packages/memory/test/memory_bench.ts +++ b/packages/memory/test/memory_bench.ts @@ -835,8 +835,57 @@ Deno.bench({ // Isolation: Merkle reference (refer) cost // -------------------------------------------------------------------------- +Deno.bench({ + name: "refer() on 4KB payload", + group: "refer-scaling", + fn(b) { + const payload = createPayload(4 * 1024); + + b.start(); + refer(payload); + b.end(); + }, +}); + Deno.bench({ name: "refer() on 16KB payload", + group: "refer-scaling", + baseline: true, + fn(b) { + const payload = createPayload(16 * 1024); + + b.start(); + refer(payload); + b.end(); + }, +}); + +Deno.bench({ + name: "refer() on 64KB payload", + group: "refer-scaling", + fn(b) { + const payload = createPayload(64 * 1024); + + b.start(); + refer(payload); + b.end(); + }, +}); + +Deno.bench({ + name: "refer() on 256KB payload", + group: "refer-scaling", + fn(b) { + const payload = createPayload(256 * 1024); + + b.start(); + refer(payload); + b.end(); + }, +}); + +Deno.bench({ + name: "refer() on 16KB payload (isolation)", group: "isolation", fn(b) { const payload = createTypicalPayload(); @@ -944,6 +993,7 @@ Deno.bench({ // Test memoization benefit: same content referenced multiple times import { refer as memoizedRefer } from "../reference.ts"; +import { unclaimedRef } from "../fact.ts"; Deno.bench({ name: "memoized: 3x refer() same payload (cache hits)", @@ -970,16 +1020,519 @@ Deno.bench({ group: "isolation", fn(b) { const doc = createDoc(); - const unclaimed = { the: "application/json", of: doc }; // Warm cache - memoizedRefer(unclaimed); + unclaimedRef({ the: "application/json" as const, of: doc }); b.start(); // Simulates multiple unclaimed refs in transaction flow + // Uses unclaimedRef() which caches by {the, of} key for (let i = 0; i < 10; i++) { - memoizedRefer({ the: "application/json", of: doc }); + unclaimedRef({ the: "application/json" as const, of: doc }); } b.end(); }, }); + +// -------------------------------------------------------------------------- +// Benchmark: Object interning +// -------------------------------------------------------------------------- + +import { intern } from "../reference.ts"; + +Deno.bench({ + name: "intern() 16KB payload (first time)", + group: "interning", + baseline: true, + fn(b) { + const payload = createTypicalPayload(); + + b.start(); + intern(payload); + b.end(); + }, +}); + +// Helper: create shared content payload (the part that's identical across objects) +function createSharedContent(targetBytes: number): object { + const base = { + type: "shared-content", + metadata: { version: 1, tags: ["data", "shared"] }, + }; + const baseSize = JSON.stringify(base).length; + const contentSize = Math.max(0, targetBytes - baseSize - 50); + return { ...base, data: "X".repeat(contentSize) }; +} + +Deno.bench({ + name: "refer() with intern() - unique IDs, shared 16KB content", + group: "interning", + fn(b) { + // Two objects with unique IDs but identical nested content + const content1 = createSharedContent(16 * 1024); + const content2 = createSharedContent(16 * 1024); // Same content, different instance + + const obj1 = { id: crypto.randomUUID(), content: content1 }; + const obj2 = { id: crypto.randomUUID(), content: content2 }; + + // Intern and refer first object + const interned1 = intern(obj1); + memoizedRefer(interned1); + + b.start(); + // Intern second object - content should be deduplicated + const interned2 = intern(obj2); + // interned2.content should === interned1.content now + memoizedRefer(interned2); // Should cache-hit on content + b.end(); + }, +}); + +Deno.bench({ + name: "refer() without intern() - unique IDs, shared 16KB content", + group: "interning", + baseline: true, + fn(b) { + // Two objects with unique IDs but identical nested content + const content1 = createSharedContent(16 * 1024); + const content2 = createSharedContent(16 * 1024); + + const obj1 = { id: crypto.randomUUID(), content: content1 }; + const obj2 = { id: crypto.randomUUID(), content: content2 }; + + // Refer first object (no interning) + memoizedRefer(obj1); + + b.start(); + // Refer second object - full re-hash (no shared identity) + memoizedRefer(obj2); + b.end(); + }, +}); + +Deno.bench({ + name: "intern() cost - unique IDs, shared 16KB content", + group: "interning", + fn(b) { + const content = createSharedContent(16 * 1024); + const obj = { id: crypto.randomUUID(), content }; + + b.start(); + intern(obj); + b.end(); + }, +}); + +Deno.bench({ + name: "intern() small object {the, of}", + group: "intern-size", + baseline: true, + fn(b) { + const doc = createDoc(); + const obj = { the: "application/json", of: doc }; + + b.start(); + intern(obj); + b.end(); + }, +}); + +Deno.bench({ + name: "intern() 1KB payload", + group: "intern-size", + fn(b) { + const payload = createPayload(1024); + + b.start(); + intern(payload); + b.end(); + }, +}); + +Deno.bench({ + name: "intern() 16KB payload", + group: "intern-size", + fn(b) { + const payload = createPayload(16 * 1024); + + b.start(); + intern(payload); + b.end(); + }, +}); + +// Compare: is interning worth it for small objects? +Deno.bench({ + name: "refer() small {the, of} - no intern", + group: "intern-small-benefit", + baseline: true, + fn(b) { + const doc = createDoc(); + + b.start(); + memoizedRefer({ the: "application/json", of: doc }); + b.end(); + }, +}); + +Deno.bench({ + name: "refer() small {the, of} - with intern (first time)", + group: "intern-small-benefit", + fn(b) { + const doc = createDoc(); + const obj = { the: "application/json", of: doc }; + + b.start(); + const interned = intern(obj); + memoizedRefer(interned); + b.end(); + }, +}); + +Deno.bench({ + name: "refer() small {the, of} - with intern (cache hit)", + group: "intern-small-benefit", + fn(b) { + const doc = "of:fixed-doc-id"; // Fixed so we get cache hits + const obj1 = { the: "application/json", of: doc }; + const obj2 = { the: "application/json", of: doc }; + + // Warm caches + const interned1 = intern(obj1); + memoizedRefer(interned1); + + b.start(); + const interned2 = intern(obj2); // Should return interned1 + memoizedRefer(interned2); // Should hit WeakMap + b.end(); + }, +}); + +// ========================================================================== +// FILE-BASED BENCHMARKS: Test real WAL/pragma impact +// ========================================================================== + +const benchDir = Deno.makeTempDirSync({ prefix: "memory-bench-" }); +let fileDbCounter = 0; + +// Helper to open a fresh file-based space +async function openFileSpace() { + // DID must be in pathname - format: file:///path/to/did:key:xxx.sqlite + const dbPath = `${benchDir}/${space.did()}-${fileDbCounter++}.sqlite`; + const result = await Space.open({ + url: new URL(`file://${dbPath}`), + }); + if (result.error) throw result.error; + return result.ok; +} + +Deno.bench({ + name: "file: set fact (single ~16KB assertion)", + group: "file-set", + baseline: true, + async fn(b) { + const session = await openFileSpace(); + warmUp(session); + + const doc = createDoc(); + const payload = createTypicalPayload(); + + b.start(); + const assertion = Fact.assert({ + the, + of: doc, + is: payload, + }); + + const transaction = Transaction.create({ + issuer: alice.did(), + subject: space.did(), + changes: Changes.from([assertion]), + }); + + const result = session.transact(transaction); + b.end(); + + if (result.error) throw result.error; + session.close(); + }, +}); + +Deno.bench({ + name: "file: set fact (10 ~16KB assertions batch)", + group: "file-set", + async fn(b) { + const session = await openFileSpace(); + warmUp(session); + + const docs = Array.from({ length: 10 }, () => createDoc()); + const payloads = Array.from({ length: 10 }, () => createTypicalPayload()); + + b.start(); + const assertions = docs.map((doc, i) => + Fact.assert({ + the, + of: doc, + is: payloads[i], + }) + ); + + const transaction = Transaction.create({ + issuer: alice.did(), + subject: space.did(), + changes: Changes.from(assertions), + }); + + const result = session.transact(transaction); + b.end(); + + if (result.error) throw result.error; + session.close(); + }, +}); + +Deno.bench({ + name: "file: set fact (100 ~16KB assertions batch)", + group: "file-set", + async fn(b) { + const session = await openFileSpace(); + warmUp(session); + + const docs = Array.from({ length: 100 }, () => createDoc()); + const payloads = Array.from({ length: 100 }, () => createTypicalPayload()); + + b.start(); + const assertions = docs.map((doc, i) => + Fact.assert({ + the, + of: doc, + is: payloads[i], + }) + ); + + const transaction = Transaction.create({ + issuer: alice.did(), + subject: space.did(), + changes: Changes.from(assertions), + }); + + const result = session.transact(transaction); + b.end(); + + if (result.error) throw result.error; + session.close(); + }, +}); + +// File-based get benchmarks +Deno.bench({ + name: "file: get fact (single ~16KB query)", + group: "file-get", + baseline: true, + async fn(b) { + const session = await openFileSpace(); + const doc = createDoc(); + + // Setup: create the fact first + const assertion = Fact.assert({ the, of: doc, is: createTypicalPayload() }); + const transaction = Transaction.create({ + issuer: alice.did(), + subject: space.did(), + changes: Changes.from([assertion]), + }); + session.transact(transaction); + + b.start(); + const query = Query.create({ + issuer: alice.did(), + subject: space.did(), + select: { [doc]: { [the]: {} } }, + }); + const result = session.query(query); + b.end(); + + if (result.error) throw result.error; + session.close(); + }, +}); + +Deno.bench({ + name: "file: get fact (wildcard query 100 ~16KB docs)", + group: "file-get", + async fn(b) { + const session = await openFileSpace(); + + // Setup: create 100 facts + const assertions = Array.from({ length: 100 }, () => + Fact.assert({ + the, + of: createDoc(), + is: createTypicalPayload(), + })); + const transaction = Transaction.create({ + issuer: alice.did(), + subject: space.did(), + changes: Changes.from(assertions), + }); + session.transact(transaction); + + b.start(); + const query = Query.create({ + issuer: alice.did(), + subject: space.did(), + select: { _: { [the]: {} } }, + }); + const result = session.query(query); + b.end(); + + if (result.error) throw result.error; + session.close(); + }, +}); + +// File-based update benchmark +Deno.bench({ + name: "file: update fact (single ~16KB)", + group: "file-update", + baseline: true, + async fn(b) { + const session = await openFileSpace(); + const doc = createDoc(); + const payload1 = createTypicalPayload(); + const payload2 = createTypicalPayload(); + + // Setup: create the initial fact + const v1 = Fact.assert({ the, of: doc, is: payload1 }); + const createTx = Transaction.create({ + issuer: alice.did(), + subject: space.did(), + changes: Changes.from([v1]), + }); + session.transact(createTx); + + b.start(); + const v2 = Fact.assert({ the, of: doc, is: payload2, cause: v1 }); + const updateTx = Transaction.create({ + issuer: alice.did(), + subject: space.did(), + changes: Changes.from([v2]), + }); + const result = session.transact(updateTx); + b.end(); + + if (result.error) throw result.error; + session.close(); + }, +}); + +Deno.bench({ + name: "file: update fact (10 sequential ~16KB updates)", + group: "file-update", + async fn(b) { + const session = await openFileSpace(); + const doc = createDoc(); + const payloads = Array.from({ length: 11 }, () => createTypicalPayload()); + + // Setup: create the initial fact + let current = Fact.assert({ the, of: doc, is: payloads[0] }); + const createTx = Transaction.create({ + issuer: alice.did(), + subject: space.did(), + changes: Changes.from([current]), + }); + session.transact(createTx); + + b.start(); + for (let i = 1; i <= 10; i++) { + const next = Fact.assert({ + the, + of: doc, + is: payloads[i], + cause: current, + }); + const updateTx = Transaction.create({ + issuer: alice.did(), + subject: space.did(), + changes: Changes.from([next]), + }); + const result = session.transact(updateTx); + if (result.error) throw result.error; + current = next; + } + b.end(); + + session.close(); + }, +}); + +// File-based workflow benchmark +Deno.bench({ + name: "file: workflow: create -> read -> update -> read -> retract", + group: "file-workflow", + async fn(b) { + const session = await openFileSpace(); + warmUp(session); + + const doc = createDoc(); + const payload1 = createTypicalPayload(); + const payload2 = createTypicalPayload(); + + b.start(); + // Create + const v1 = Fact.assert({ the, of: doc, is: payload1 }); + const createResult = session.transact( + Transaction.create({ + issuer: alice.did(), + subject: space.did(), + changes: Changes.from([v1]), + }), + ); + + // Read + const readResult1 = session.query( + Query.create({ + issuer: alice.did(), + subject: space.did(), + select: { [doc]: { [the]: {} } }, + }), + ); + + // Update + const v2 = Fact.assert({ the, of: doc, is: payload2, cause: v1 }); + const updateResult = session.transact( + Transaction.create({ + issuer: alice.did(), + subject: space.did(), + changes: Changes.from([v2]), + }), + ); + + // Read again + const readResult2 = session.query( + Query.create({ + issuer: alice.did(), + subject: space.did(), + select: { [doc]: { [the]: {} } }, + }), + ); + + // Retract + const r = Fact.retract(v2); + const retractResult = session.transact( + Transaction.create({ + issuer: alice.did(), + subject: space.did(), + changes: Changes.from([r]), + }), + ); + b.end(); + + if (createResult.error) throw createResult.error; + if (readResult1.error) throw readResult1.error; + if (updateResult.error) throw updateResult.error; + if (readResult2.error) throw readResult2.error; + if (retractResult.error) throw retractResult.error; + + session.close(); + }, +}); From 6e979d0b0c021877882e8f3c13262073bcf36103 Mon Sep 17 00:00:00 2001 From: Jordan Santell Date: Fri, 5 Dec 2025 11:55:18 -0800 Subject: [PATCH 2/3] chore: shell: Separate the derivation of active pattern and space root pattern, syncing state (#2188) chore: shell: Separate the derivation of active pattern and space root pattern, synchronizing the UI state Rewrite some pattern integration tests to be less dependent on timing issues, surfaced by this change. instantiate-recipe.test.ts has been disabled for now. --- packages/charm/src/ops/charms-controller.ts | 8 +- packages/patterns/integration/counter.test.ts | 50 +- .../patterns/integration/ct-checkbox.test.ts | 79 +-- packages/patterns/integration/ct-list.test.ts | 364 +++++-------- .../patterns/integration/ct-render.test.ts | 37 +- packages/patterns/integration/ct-tags.test.ts | 485 ++++++------------ .../integration/instantiate-recipe.test.ts | 119 +++-- .../integration/list-operations.test.ts | 66 ++- .../integration/nested-counter.test.ts | 17 +- packages/shell/integration/charm.test.ts | 27 +- .../iframe-counter-charm.disabled_test.ts | 5 +- packages/shell/src/lib/pattern-factory.ts | 19 +- packages/shell/src/lib/runtime.ts | 67 ++- packages/shell/src/views/AppView.ts | 182 ++++--- packages/shell/src/views/BodyView.ts | 14 +- packages/shell/src/views/RootView.ts | 7 +- 16 files changed, 659 insertions(+), 887 deletions(-) diff --git a/packages/charm/src/ops/charms-controller.ts b/packages/charm/src/ops/charms-controller.ts index 56eecb7a2a..84ab85f44e 100644 --- a/packages/charm/src/ops/charms-controller.ts +++ b/packages/charm/src/ops/charms-controller.ts @@ -29,14 +29,14 @@ export class CharmsController { return this.#manager; } - async create( + async create( program: RuntimeProgram | string, options: CreateCharmOptions = {}, cause: string | undefined = undefined, - ): Promise> { + ): Promise> { this.disposeCheck(); const recipe = await compileProgram(this.#manager, program); - const charm = await this.#manager.runPersistent( + const charm = await this.#manager.runPersistent( recipe, options.input, cause, @@ -45,7 +45,7 @@ export class CharmsController { ); await this.#manager.runtime.idle(); await this.#manager.synced(); - return new CharmController(this.#manager, charm); + return new CharmController(this.#manager, charm); } async get( diff --git a/packages/patterns/integration/counter.test.ts b/packages/patterns/integration/counter.test.ts index 1a0e55311f..e18b61d52d 100644 --- a/packages/patterns/integration/counter.test.ts +++ b/packages/patterns/integration/counter.test.ts @@ -3,7 +3,7 @@ import { CharmController, CharmsController } from "@commontools/charm/ops"; import { ShellIntegration } from "@commontools/integration/shell-utils"; import { afterAll, beforeAll, describe, it } from "@std/testing/bdd"; import { join } from "@std/path"; -import { assert, assertEquals } from "@std/assert"; +import { assertEquals } from "@std/assert"; import { Identity } from "@commontools/identity"; import { FileSystemProgramResolver } from "@commontools/js-compiler"; @@ -50,16 +50,16 @@ describe("counter direct operations test", () => { identity, }); - const counterResult = await page.waitForSelector("#counter-result", { - strategy: "pierce", - }); - assert(counterResult, "Should find counter-result element"); - // Verify initial value is 0 - const initialText = await counterResult.evaluate((el: HTMLElement) => - el.textContent - ); - assertEquals(initialText?.trim(), "Counter is the 0th number"); + await waitFor(async () => { + const counterResult = await page.waitForSelector("#counter-result", { + strategy: "pierce", + }); + const initialText = await counterResult.evaluate((el: HTMLElement) => + el.textContent + ); + return initialText?.trim() === "Counter is the 0th number"; + }); assertEquals(charm.result.get(["value"]), 0); }); @@ -67,15 +67,17 @@ describe("counter direct operations test", () => { it("should update counter value via direct operation (live)", async () => { const page = shell.page(); - // Get the counter result element - const counterResult = await page.waitForSelector("#counter-result", { + await page.waitForSelector("#counter-result", { strategy: "pierce", }); - console.log("Setting counter value to 42 via direct operation"); await charm.result.set(42, ["value"]); await waitFor(async () => { + const counterResult = await page.waitForSelector("#counter-result", { + strategy: "pierce", + }); + const updatedText = await counterResult.evaluate((el: HTMLElement) => el.textContent ); @@ -105,19 +107,15 @@ describe("counter direct operations test", () => { }); // Get the counter result element after refresh - const counterResult = await page.waitForSelector("#counter-result", { - strategy: "pierce", - }); - assert(counterResult, "Should find counter-result element after refresh"); + await waitFor(async () => { + const counterResult = await page.waitForSelector("#counter-result", { + strategy: "pierce", + }); - // Check if the UI shows the updated value after refresh - const textAfterRefresh = await counterResult.evaluate((el: HTMLElement) => - el.textContent - ); - assertEquals( - textAfterRefresh?.trim(), - "Counter is the 42th number", - "UI should show persisted value after refresh", - ); + const textAfterRefresh = await counterResult.evaluate((el: HTMLElement) => + el.textContent + ); + return textAfterRefresh?.trim() === "Counter is the 42th number"; + }); }); }); diff --git a/packages/patterns/integration/ct-checkbox.test.ts b/packages/patterns/integration/ct-checkbox.test.ts index 24d0071e8c..759388039d 100644 --- a/packages/patterns/integration/ct-checkbox.test.ts +++ b/packages/patterns/integration/ct-checkbox.test.ts @@ -1,9 +1,7 @@ -import { env } from "@commontools/integration"; -import { sleep } from "@commontools/utils/sleep"; +import { env, waitFor } from "@commontools/integration"; import { ShellIntegration } from "@commontools/integration/shell-utils"; import { afterAll, beforeAll, describe, it } from "@std/testing/bdd"; import { join } from "@std/path"; -import { assertEquals } from "@std/assert"; import { Identity } from "@commontools/identity"; import { CharmsController } from "@commontools/charm/ops"; import { ANYONE_USER } from "@commontools/memory/acl"; @@ -67,49 +65,62 @@ testComponents.forEach(({ name, file }) => { it("should show disabled content initially", async () => { const page = shell.page(); - const featureStatus = await page.waitForSelector("#feature-status", { - strategy: "pierce", + await waitFor(async () => { + const featureStatus = await page.waitForSelector("#feature-status", { + strategy: "pierce", + }); + const statusText = await featureStatus.evaluate((el: HTMLElement) => + el.textContent + ); + return statusText?.trim() === "⚠ Feature is disabled"; }); - const statusText = await featureStatus.evaluate((el: HTMLElement) => - el.textContent - ); - assertEquals(statusText?.trim(), "⚠ Feature is disabled"); }); it("should toggle to enabled content when checkbox is clicked", async () => { const page = shell.page(); - const checkbox = await page.waitForSelector("ct-checkbox", { - strategy: "pierce", - }); - await checkbox.click(); - await sleep(500); - - const featureStatus = await page.$("#feature-status", { - strategy: "pierce", + await waitFor(async () => { + const checkbox = await page.waitForSelector("ct-checkbox", { + strategy: "pierce", + }); + // This could throw due to lacking a box model to click on. + // Catch in lieu of handling time sensitivity. + try { + await checkbox.click(); + } catch (_) { + return false; + } + const featureStatus = await page.waitForSelector("#feature-status", { + strategy: "pierce", + }); + const statusText = await featureStatus.evaluate((el: HTMLElement) => + el.textContent + ); + return statusText?.trim() === "✓ Feature is enabled!"; }); - const statusText = await featureStatus?.evaluate((el: HTMLElement) => - el.textContent - ); - assertEquals(statusText?.trim(), "✓ Feature is enabled!"); }); it("should toggle back to disabled content when checkbox is clicked again", async () => { const page = shell.page(); - - const checkbox = await page.$("ct-checkbox", { - strategy: "pierce", - }); - await checkbox?.click(); - await sleep(1000); - - const featureStatus = await page.$("#feature-status", { - strategy: "pierce", + await waitFor(async () => { + const checkbox = await page.waitForSelector("ct-checkbox", { + strategy: "pierce", + }); + // This could throw due to lacking a box model to click on. + // Catch in lieu of handling time sensitivity. + try { + await checkbox.click(); + } catch (_) { + return false; + } + const featureStatus = await page.waitForSelector("#feature-status", { + strategy: "pierce", + }); + const statusText = await featureStatus.evaluate((el: HTMLElement) => + el.textContent + ); + return statusText?.trim() === "⚠ Feature is disabled"; }); - const statusText = await featureStatus?.evaluate((el: HTMLElement) => - el.textContent - ); - assertEquals(statusText?.trim(), "⚠ Feature is disabled"); }); }); }); diff --git a/packages/patterns/integration/ct-list.test.ts b/packages/patterns/integration/ct-list.test.ts index 6a9f5d6103..45556d7c40 100644 --- a/packages/patterns/integration/ct-list.test.ts +++ b/packages/patterns/integration/ct-list.test.ts @@ -1,11 +1,11 @@ -import { env } from "@commontools/integration"; -import { sleep } from "@commontools/utils/sleep"; +import { env, Page, waitFor } from "@commontools/integration"; import { ShellIntegration } from "@commontools/integration/shell-utils"; import { afterAll, beforeAll, describe, it } from "@std/testing/bdd"; import { join } from "@std/path"; import { assert, assertEquals } from "@std/assert"; import { Identity } from "@commontools/identity"; import { CharmsController } from "@commontools/charm/ops"; +import { ElementHandle } from "@astral/astral"; const { API_URL, FRONTEND_URL, SPACE_NAME } = env; @@ -56,102 +56,30 @@ describe("ct-list integration test", () => { it("should add items to the list", async () => { const page = shell.page(); + const list = new List(page); - // Find the add item input in ct-list - const addInput = await page.waitForSelector(".add-item-input", { - strategy: "pierce", - }); - - // Add first item - await addInput.click(); - await addInput.type("First item"); - await page.keyboard.press("Enter"); - await sleep(50); // Quick wait for DOM update - - // Add second item - the input should be cleared automatically - await addInput.type("Second item"); - await page.keyboard.press("Enter"); - await sleep(50); // Quick wait for DOM update - - // Add third item - await addInput.type("Third item"); - await page.keyboard.press("Enter"); - await sleep(50); // Quick wait for DOM update - - // Verify items were added - const listItems = await page.$$(".list-item", { strategy: "pierce" }); - assertEquals(listItems.length, 3, "Should have 3 items in the list"); - - // Debug: Log the structure of list items - console.log("List item structure:"); - for (let i = 0; i < listItems.length; i++) { - const itemInfo = await listItems[i].evaluate( - (el: HTMLElement, idx: number) => { - const buttons = el.querySelectorAll("button"); - return { - index: idx, - className: el.className, - innerText: el.innerText, - buttonCount: buttons.length, - buttons: Array.from(buttons).map((b) => ({ - className: b.className, - title: b.title || "no title", - innerText: b.innerText, - })), - }; - }, - { args: [i] } as any, - ); - console.log(`Item ${i}:`, itemInfo); - } + await list.addItem("First item"); + await list.waitForItemCount(1); - // Quick wait for content to render - await sleep(100); - - // Verify item content - const firstItemText = await listItems[0].evaluate((el: HTMLElement) => { - const content = el.querySelector(".item-content") || - el.querySelector("div.item-content"); - return content?.textContent || el.textContent; - }); - assertEquals(firstItemText?.trim(), "First item"); + await list.addItem("Second item"); + await list.waitForItemCount(2); - const secondItemText = await listItems[1].evaluate((el: HTMLElement) => { - const content = el.querySelector(".item-content") || - el.querySelector("div.item-content"); - return content?.textContent || el.textContent; - }); - assertEquals(secondItemText?.trim(), "Second item"); + await list.addItem("Third item"); + await list.waitForItemCount(3); - const thirdItemText = await listItems[2].evaluate((el: HTMLElement) => { - const content = el.querySelector(".item-content") || - el.querySelector("div.item-content"); - return content?.textContent || el.textContent; - }); - assertEquals(thirdItemText?.trim(), "Third item"); + assertEquals(await list.getItemsText(), [ + "First item", + "Second item", + "Third item", + ]); }); it("should update the list title", async () => { const page = shell.page(); + const list = new List(page); - // Find the title input - const titleInput = await page.$("input[placeholder='List title']", { - strategy: "pierce", - }); - assert(titleInput, "Should find title input"); - - // Clear the existing text first - await titleInput.click(); - await titleInput.evaluate((el: HTMLInputElement) => { - el.select(); // Select all text - }); - await titleInput.type("My Shopping List"); - - // Verify title was updated (no wait needed for input value) - const titleValue = await titleInput.evaluate((el: HTMLInputElement) => - el.value - ); - assertEquals(titleValue, "My Shopping List"); + await list.setTitle("My Shopping List"); + assertEquals(await list.getTitle(), "My Shopping List"); }); // TODO(#CT-703): Fix this test - there's a bug where programmatic clicks on the remove button @@ -160,142 +88,85 @@ describe("ct-list integration test", () => { // versus real user clicks. it("should remove items from the list", async () => { const page = shell.page(); + const list = new List(page); - console.log("Waiting for component to stabilize..."); - await sleep(500); - - // Get initial count - const initialItems = await page.$$(".list-item", { strategy: "pierce" }); - const initialCount = initialItems.length; - console.log(`Initial item count: ${initialCount}`); - assert(initialCount > 0, "Should have items to remove"); - - // Find and click the first remove button - const removeButtons = await page.$$("button.item-action.remove", { - strategy: "pierce", - }); - console.log(`Found ${removeButtons.length} remove buttons`); - assert(removeButtons.length > 0, "Should find remove buttons"); - - // Debug: check what we're about to click - const buttonText = await removeButtons[0].evaluate((el: HTMLElement) => { - return { - className: el.className, - title: el.title, - innerText: el.innerText, - parentText: el.parentElement?.innerText || "no parent", - }; - }); - console.log("About to click button:", buttonText); + const items = await list.getItems(); + assert(items.length > 0, "Should have items to remove"); + const initialCount = items.length; - // Try clicking more carefully - console.log("Waiting before click..."); - await sleep(100); + await list.removeItem(items[0]); + await list.waitForItemCount(initialCount - 1); - // Alternative approach: dispatch click event - await removeButtons[0].evaluate((button: HTMLElement) => { - console.log("About to dispatch click event on button:", button); - button.dispatchEvent( - new MouseEvent("click", { - bubbles: true, - cancelable: true, - view: window, - }), - ); - }); - console.log("Dispatched click event on first remove button"); + assertEquals(await list.getItemsText(), [ + "Second item", + "Third item", + ]); + }); - // Check immediately after click - await sleep(50); - const immediateItems = await page.$$(".list-item", { strategy: "pierce" }); - console.log( - `Immediately after click, found ${immediateItems.length} items`, - ); + it("should edit items in the list", async () => { + const page = shell.page(); + const list = new List(page); - // Wait for DOM to update after removal using Astral's waitForFunction - await page.waitForFunction((expectedCount) => { - const items = document.querySelectorAll(".list-item"); - return items.length !== expectedCount; - }, { args: [initialCount] }); - - // Verify item was removed - try multiple times - let remainingItems = await page.$$(".list-item", { strategy: "pierce" }); - console.log(`After removal, found ${remainingItems.length} items`); - - // If still showing same count, wait a bit more and try again - if (remainingItems.length === initialCount) { - console.log("DOM not updated yet, waiting more..."); - await sleep(500); - remainingItems = await page.$$(".list-item", { strategy: "pierce" }); - console.log( - `After additional wait, found ${remainingItems.length} items`, - ); - } + const items = await list.getItems(); + assert(items.length > 0, "Should have items to edit"); - assertEquals( - remainingItems.length, - initialCount - 1, - "Should have one less item after removal", + const newText = "Edited Second Item"; + await list.editItem(items[0], newText); + await waitFor(() => + list.getItems().then((els) => list.getItemText(els[0])).then((text) => + text === newText + ) ); - // Verify the first item is now what was the second item - if (remainingItems.length > 0) { - const firstRemainingText = await remainingItems[0].evaluate( - (el: HTMLElement) => { - const content = el.querySelector(".item-content") || - el.querySelector("div.item-content"); - return content?.textContent || el.textContent; - }, - ); - assertEquals( - firstRemainingText?.trim(), - "Second item", - "First item should now be the second item", - ); - } + assertEquals(await list.getItemsText(), [ + "Edited Second Item", + "Third item", + ]); }); +}); - it("should edit items in the list", async () => { - const page = shell.page(); +class List { + #page: Page; + constructor(page: Page) { + this.#page = page; + } - console.log("Waiting for component to stabilize..."); - await sleep(500); + getItems(): Promise { + return this.#page.$$(".list-item", { strategy: "pierce" }); + } - // Get initial items - const initialItems = await page.$$(".list-item", { strategy: "pierce" }); - const initialCount = initialItems.length; - console.log(`Initial item count: ${initialCount}`); - assert(initialCount > 0, "Should have items to edit"); + async getItemsText(): Promise> { + const elements = await this.getItems(); + return Promise.all(elements.map((el) => this.getItemText(el))); + } - // Get the initial text of the first item - const initialText = await initialItems[0].evaluate((el: HTMLElement) => { + async getItemText(element: ElementHandle): Promise { + return await element.evaluate((el: HTMLElement) => { const content = el.querySelector(".item-content") || el.querySelector("div.item-content"); - return content?.textContent || el.textContent; + return (content?.textContent || el.textContent)?.trim(); }); - console.log(`Initial text of first item: "${initialText?.trim()}"`); + } + + async waitForItemCount(expected: number): Promise { + await waitFor(() => this.getItems().then((els) => els.length === expected)); + } - // Find and click the first edit button - const editButtons = await page.$$("button.item-action.edit", { + async addItem(text: string): Promise { + const addInput = await this.#page.waitForSelector(".add-item-input", { strategy: "pierce", }); - console.log(`Found ${editButtons.length} edit buttons`); - assert(editButtons.length > 0, "Should find edit buttons"); - - // Debug: check what we're about to click - const buttonText = await editButtons[0].evaluate((el: HTMLElement) => { - return { - className: el.className, - title: el.title, - innerText: el.innerText, - parentText: el.parentElement?.innerText || "no parent", - }; - }); - console.log("About to click edit button:", buttonText); + await addInput.click(); + await addInput.type(text); + await this.#page.keyboard.press("Enter"); + } + + async removeItem(element: ElementHandle): Promise { + // Find the remove button within this item + const removeButton = await element.$("button.item-action.remove"); + assert(removeButton, "Should find remove button in item"); - // Click the edit button to enter edit mode - await editButtons[0].evaluate((button: HTMLElement) => { - console.log("About to dispatch click event on edit button:", button); + await removeButton.evaluate((button: HTMLElement) => { button.dispatchEvent( new MouseEvent("click", { bubbles: true, @@ -304,59 +175,56 @@ describe("ct-list integration test", () => { }), ); }); - console.log("Dispatched click event on first edit button"); + } - // Wait for edit mode to activate and look for the editing state - await page.waitForSelector(".list-item.editing", { strategy: "pierce" }); - console.log("Edit mode activated - found .list-item.editing"); + async editItem(element: ElementHandle, newText: string): Promise { + // Find the edit button within this item + const editButton = await element.$("button.item-action.edit"); + assert(editButton, "Should find edit button in item"); - // Look for the specific edit input field that appears only during editing - const editInput = await page.$(".edit-input", { - strategy: "pierce", + await editButton.evaluate((button: HTMLElement) => { + button.dispatchEvent( + new MouseEvent("click", { + bubbles: true, + cancelable: true, + view: window, + }), + ); }); - assert(editInput, "Should find .edit-input field during editing"); - // Verify the input is focused (it should have autofocus) - const isFocused = await editInput.evaluate((el: HTMLInputElement) => - document.activeElement === el - ); - console.log(`Edit input is focused: ${isFocused}`); + // Wait for edit mode to activate + await this.#page.waitForSelector(".list-item.editing", { + strategy: "pierce", + }); - // Clear the existing text and type new text + // Find the edit input and type new text + const editInput = await this.#page.waitForSelector(".edit-input", { + strategy: "pierce", + }); await editInput.evaluate((el: HTMLInputElement) => { - el.select(); // Select all text + el.select(); }); - const newText = "Edited First Item"; await editInput.type(newText); - console.log(`Typed new text: "${newText}"`); + await this.#page.keyboard.press("Enter"); + } - // Press Enter to confirm the edit - await page.keyboard.press("Enter"); - console.log("Pressed Enter to confirm edit"); - - // Wait for the edit to be processed - await sleep(200); - - // Verify the item was edited - const updatedItems = await page.$$(".list-item", { strategy: "pierce" }); - assertEquals( - updatedItems.length, - initialCount, - "Should have same number of items after edit", + async setTitle(title: string): Promise { + const titleInput = await this.#page.waitForSelector( + "input[placeholder='List title']", + { strategy: "pierce" }, ); - - // Check that the first item's text has been updated - const updatedText = await updatedItems[0].evaluate((el: HTMLElement) => { - const content = el.querySelector(".item-content") || - el.querySelector("div.item-content"); - return content?.textContent || el.textContent; + await titleInput.click(); + await titleInput.evaluate((el: HTMLInputElement) => { + el.select(); }); - console.log(`Updated text of first item: "${updatedText?.trim()}"`); + await titleInput.type(title); + } - assertEquals( - updatedText?.trim(), - newText, - "First item should have updated text", + async getTitle(): Promise { + const titleInput = await this.#page.waitForSelector( + "input[placeholder='List title']", + { strategy: "pierce" }, ); - }); -}); + return await titleInput.evaluate((el: HTMLInputElement) => el.value); + } +} diff --git a/packages/patterns/integration/ct-render.test.ts b/packages/patterns/integration/ct-render.test.ts index d5545d45cf..56aad22a6a 100644 --- a/packages/patterns/integration/ct-render.test.ts +++ b/packages/patterns/integration/ct-render.test.ts @@ -51,16 +51,15 @@ describe("ct-render integration test", () => { identity, }); - const counterResult = await page.waitForSelector("#counter-result", { - strategy: "pierce", + await waitFor(async () => { + const counterResult = await page.waitForSelector("#counter-result", { + strategy: "pierce", + }); + const initialText = await counterResult.evaluate((el: HTMLElement) => + el.textContent + ); + return initialText?.trim() === "Counter is the 0th number"; }); - assert(counterResult, "Should find counter-result element"); - - // Verify initial value is 0 - const initialText = await counterResult.evaluate((el: HTMLElement) => - el.textContent - ); - assertEquals(initialText?.trim(), "Counter is the 0th number"); // Verify via direct operations that the ct-render structure works const value = charm.result.get(["value"]); @@ -107,18 +106,16 @@ describe("ct-render integration test", () => { identity, }); - // Check if the UI shows the updated value - const counterResult = await page.waitForSelector("#counter-result", { - strategy: "pierce", + await waitFor(async () => { + const counterResult = await page.waitForSelector("#counter-result", { + strategy: "pierce", + }); + const textAfterUpdate = await counterResult.evaluate((el: HTMLElement) => + el.textContent + ); + return textAfterUpdate?.trim() === + "Counter is the 5th number"; }); - const textAfterUpdate = await counterResult.evaluate((el: HTMLElement) => - el.textContent - ); - assertEquals( - textAfterUpdate?.trim(), - "Counter is the 5th number", - "UI should show updated value from direct operation", - ); }); it("should verify only ONE counter display", async () => { diff --git a/packages/patterns/integration/ct-tags.test.ts b/packages/patterns/integration/ct-tags.test.ts index 82c7b90e31..19ae0ac968 100644 --- a/packages/patterns/integration/ct-tags.test.ts +++ b/packages/patterns/integration/ct-tags.test.ts @@ -1,11 +1,12 @@ -import { env } from "@commontools/integration"; +import { env, Page, waitFor } from "@commontools/integration"; import { sleep } from "@commontools/utils/sleep"; import { ShellIntegration } from "@commontools/integration/shell-utils"; import { afterAll, beforeAll, describe, it } from "@std/testing/bdd"; import { join } from "@std/path"; -import { assert, assertEquals } from "@std/assert"; +import { assertEquals } from "@std/assert"; import { Identity } from "@commontools/identity"; import { CharmsController } from "@commontools/charm/ops"; +import { ElementHandle } from "@astral/astral"; const { API_URL, FRONTEND_URL, SPACE_NAME } = env; @@ -56,212 +57,188 @@ describe("ct-tags integration test", () => { it("should add tags to the list", async () => { const page = shell.page(); + const tags = new Tags(page); + await tags.addTag("frontend"); + await tags.waitForTagCount(1); + await tags.addTag("javascript"); + await tags.waitForTagCount(2); + await tags.addTag("testing"); + await tags.waitForTagCount(3); + assertEquals(await tags.getTagsText(), [ + "frontend", + "javascript", + "testing", + ]); + }); - // Helper function to add a tag - const addTag = async (tagText: string) => { - // Find the add tag button - const addTagButton = await page.waitForSelector(".add-tag", { - strategy: "pierce", - }); - - // Click using evaluate to ensure it works - await addTagButton.evaluate((el: HTMLElement) => { - el.click(); - }); - await sleep(100); // Wait for input to appear - - const addInput = await page.waitForSelector(".add-tag-input", { - strategy: "pierce", - }); - await addInput.type(tagText); - await page.keyboard.press("Enter"); - await sleep(100); // Wait for DOM update - }; - - // Add first tag - await addTag("frontend"); - - // Add second tag - await addTag("javascript"); + it("should not add duplicate tags", async () => { + const page = shell.page(); + const tags = new Tags(page); + assertEquals((await tags.getTags()).length, 3); - // Add third tag - await addTag("testing"); + // Try to add a duplicate tag + await tags.addTag("frontend"); + await tags.waitForTagCount(3); + assertEquals(await tags.getTagsText(), [ + "frontend", + "javascript", + "testing", + ]); + }); - // Verify tags were added - const tags = await page.$$(".tag", { strategy: "pierce" }); - assertEquals(tags.length, 3, "Should have 3 tags"); + it("should edit tags", async () => { + const page = shell.page(); + const tags = new Tags(page); + assertEquals( + (await tags.getTags()).length, + 3, + "Should still have 3 tags (no duplicates)", + ); - // Debug: Log the structure of tags - console.log("Tag structure:"); - for (let i = 0; i < tags.length; i++) { - const tagInfo = await tags[i].evaluate( - (el: HTMLElement, idx: number) => { - const tagText = el.querySelector(".tag-text"); - const removeButton = el.querySelector(".tag-remove"); - return { - index: idx, - className: el.className, - tagText: tagText?.textContent || "no text", - hasRemoveButton: !!removeButton, - }; - }, - { args: [i] } as any, - ); - console.log(`Tag ${i}:`, tagInfo); - } + const newText = "backend"; + const tagEls = await tags.getTags(); + await tags.editTag(tagEls[0], newText); + await waitFor(() => + tags.getTags().then((els) => tags.getTagText(els[0])).then((text) => + text === newText + ) + ); + assertEquals(await tags.getTagsText(), [ + "backend", + "javascript", + "testing", + ]); + }); - // Verify tag content - const firstTagText = await tags[0].evaluate((el: HTMLElement) => { - const tagText = el.querySelector(".tag-text"); - return tagText?.textContent || el.textContent; - }); - assertEquals(firstTagText?.trim(), "frontend"); + it("should remove tags", async () => { + const page = shell.page(); + const tags = new Tags(page); + assertEquals((await tags.getTags()).length, 3); + const elements = await tags.getTags(); + await tags.removeTag(elements[0]); + await tags.waitForTagCount(2); + assertEquals(await tags.getTagsText(), [ + "javascript", + "testing", + ]); + }); - const secondTagText = await tags[1].evaluate((el: HTMLElement) => { - const tagText = el.querySelector(".tag-text"); - return tagText?.textContent || el.textContent; - }); - assertEquals(secondTagText?.trim(), "javascript"); + it("should cancel tag editing with Escape key", async () => { + const page = shell.page(); + const tags = new Tags(page); + assertEquals((await tags.getTags()).length, 2); + const originalTexts = await tags.getTagsText(); - const thirdTagText = await tags[2].evaluate((el: HTMLElement) => { - const tagText = el.querySelector(".tag-text"); - return tagText?.textContent || el.textContent; + const elements = await tags.getTags(); + const element = elements[0]; + await element.click(); + await page.waitForSelector(".tag.editing", { strategy: "pierce" }); + const editInput = await page.waitForSelector(".tag-input", { + strategy: "pierce", }); - assertEquals(thirdTagText?.trim(), "testing"); + await editInput.evaluate((el: HTMLInputElement) => el.select()); + await editInput.type("should-be-cancelled"); + await page.keyboard.press("Escape"); + await sleep(100); + assertEquals(await tags.getTagsText(), originalTexts); }); - it("should not add duplicate tags", async () => { + it("should delete empty tags when backspacing", async () => { const page = shell.page(); + const tags = new Tags(page); + assertEquals((await tags.getTags()).length, 2); - // Helper function to add a tag - const addTag = async (tagText: string) => { - const addTagButton = await page.waitForSelector(".add-tag", { - strategy: "pierce", - }); - - await addTagButton.evaluate((el: HTMLElement) => { - el.click(); - }); - await sleep(100); - - const addInput = await page.waitForSelector(".add-tag-input", { - strategy: "pierce", - }); - await addInput.type(tagText); - await page.keyboard.press("Enter"); - await sleep(100); - }; - - // Try to add a duplicate tag - await addTag("frontend"); // This already exists + const elements = await tags.getTags(); + const element = elements[0]; + await element.click(); + await page.waitForSelector(".tag.editing", { strategy: "pierce" }); + const editInput = await page.waitForSelector(".tag-input", { + strategy: "pierce", + }); + await editInput.evaluate((el: HTMLInputElement) => el.select()); + await page.keyboard.press("Delete"); + await sleep(50); // Wait for input to be cleared + // Press Backspace on empty input + await page.keyboard.press("Backspace"); + await sleep(200); - // Should still have 3 tags, not 4 - const tags = await page.$$(".tag", { strategy: "pierce" }); - assertEquals(tags.length, 3, "Should still have 3 tags (no duplicates)"); + await tags.waitForTagCount(1); }); +}); - it("should edit tags", async () => { - const page = shell.page(); - - console.log("Waiting for component to stabilize..."); - await sleep(500); - - // Get initial tags - const initialTags = await page.$$(".tag", { strategy: "pierce" }); - const initialCount = initialTags.length; - console.log(`Initial tag count: ${initialCount}`); - assert(initialCount > 0, "Should have tags to edit"); +class Tags { + #page: Page; + constructor(page: Page) { + this.#page = page; + } - // Click on the first tag to edit it - await initialTags[0].click(); - console.log("Clicked on first tag"); + getTags(): Promise { + return this.#page.$$(".tag", { strategy: "pierce" }); + } - // Wait for edit mode to activate - await page.waitForSelector(".tag.editing", { strategy: "pierce" }); - console.log("Edit mode activated - found .tag.editing"); + async getTagsText(): Promise> { + const elements = await this.getTags(); + return Promise.all(elements.map((el) => this.getTagText(el))); + } + async editTag(element: ElementHandle, newText: string) { + await element.click(); + await this.#page.waitForSelector(".tag.editing", { strategy: "pierce" }); // Look for the tag input field that appears during editing - const editInput = await page.waitForSelector(".tag-input", { + const editInput = await this.#page.waitForSelector(".tag-input", { strategy: "pierce", }); - assert(editInput, "Should find .tag-input field during editing"); // Clear and type new text await editInput.evaluate((el: HTMLInputElement) => { el.select(); // Select all text }); - const newText = "backend"; await editInput.type(newText); - console.log(`Typed new text: "${newText}"`); - - // Press Enter to confirm the edit - await page.keyboard.press("Enter"); - console.log("Pressed Enter to confirm edit"); - - // Wait for the edit to be processed - await sleep(200); - - // Verify the tag was edited - const updatedTags = await page.$$(".tag", { strategy: "pierce" }); - assertEquals( - updatedTags.length, - initialCount, - "Should have same number of tags after edit", + await this.#page.keyboard.press("Enter"); + } + + async waitForTagCount(expected: number): Promise { + await waitFor(() => this.getTags().then((els) => els.length === expected)); + } + + async waitForTagText(element: ElementHandle, text: string): Promise { + await waitFor(() => + element.evaluate((el: HTMLInputElement) => el.value).then((value) => + value === text + ) ); + } - // Check that the first tag's text has been updated - const updatedText = await updatedTags[0].evaluate((el: HTMLElement) => { + async getTagText(element: ElementHandle): Promise { + return await element.evaluate((el: HTMLElement) => { const tagText = el.querySelector(".tag-text"); return tagText?.textContent || el.textContent; }); - console.log(`Updated text of first tag: "${updatedText?.trim()}"`); + } - assertEquals( - updatedText?.trim(), - newText, - "First tag should have updated text", - ); - }); - - it("should remove tags", async () => { - const page = shell.page(); - - console.log("Waiting for component to stabilize..."); - await sleep(500); - - // Get initial count - const initialTags = await page.$$(".tag", { strategy: "pierce" }); - const initialCount = initialTags.length; - console.log(`Initial tag count: ${initialCount}`); - assert(initialCount > 0, "Should have tags to remove"); + async addTag(text: string) { + const addTagButton = await this.#page.waitForSelector(".add-tag", { + strategy: "pierce", + }); + await addTagButton.click(); + const addInput = await this.#page.waitForSelector(".add-tag-input", { + strategy: "pierce", + }); + await addInput.type(text); + await this.#page.keyboard.press("Enter"); + } + async removeTag(element: ElementHandle): Promise { // Hover over the first tag to make the remove button visible - await initialTags[0].evaluate((el: HTMLElement) => { + await element.evaluate((el: HTMLElement) => { el.dispatchEvent(new MouseEvent("mouseover", { bubbles: true })); }); - await sleep(100); // Wait for hover effect // Find and click the first remove button - const removeButtons = await page.$$(".tag-remove", { + const removeButtons = await this.#page.waitForSelector(".tag-remove", { strategy: "pierce", }); - console.log(`Found ${removeButtons.length} remove buttons`); - assert(removeButtons.length > 0, "Should find remove buttons"); - - // Debug: check what we're about to click - const buttonInfo = await removeButtons[0].evaluate((el: HTMLElement) => { - return { - className: el.className, - title: el.title, - innerText: el.innerText, - parentText: el.parentElement?.innerText || "no parent", - }; - }); - console.log("About to click remove button:", buttonInfo); - - // Click the remove button - await removeButtons[0].evaluate((button: HTMLElement) => { - console.log("About to dispatch click event on remove button:", button); + await removeButtons.evaluate((button: HTMLElement) => { button.dispatchEvent( new MouseEvent("click", { bubbles: true, @@ -270,175 +247,5 @@ describe("ct-tags integration test", () => { }), ); }); - console.log("Dispatched click event on first remove button"); - - // Wait for DOM to update after removal - await sleep(200); - - // Verify tag was removed - const remainingTags = await page.$$(".tag", { strategy: "pierce" }); - console.log(`After removal, found ${remainingTags.length} tags`); - - assertEquals( - remainingTags.length, - initialCount - 1, - "Should have one less tag after removal", - ); - - // Verify the first tag is now what was the second tag - if (remainingTags.length > 0) { - const firstRemainingText = await remainingTags[0].evaluate( - (el: HTMLElement) => { - const tagText = el.querySelector(".tag-text"); - return tagText?.textContent || el.textContent; - }, - ); - assertEquals( - firstRemainingText?.trim(), - "javascript", - "First tag should now be the second tag", - ); - } - }); - - it("should cancel tag editing with Escape key", async () => { - const page = shell.page(); - - console.log("Waiting for component to stabilize..."); - await sleep(500); - - // Helper function to add a tag if needed - const addTag = async (tagText: string) => { - const addTagButton = await page.waitForSelector(".add-tag", { - strategy: "pierce", - }); - - await addTagButton.evaluate((el: HTMLElement) => { - el.click(); - }); - await sleep(100); - - const addInput = await page.waitForSelector(".add-tag-input", { - strategy: "pierce", - }); - await addInput.type(tagText); - await page.keyboard.press("Enter"); - await sleep(100); - }; - - let tags = await page.$$(".tag", { strategy: "pierce" }); - - // Add a tag if none exist - if (tags.length === 0) { - await addTag("test-tag"); - tags = await page.$$(".tag", { strategy: "pierce" }); - } - - assert(tags.length > 0, "Should have tags to test escape behavior"); - - // Get the original text of the first tag - const originalText = await tags[0].evaluate((el: HTMLElement) => { - const tagText = el.querySelector(".tag-text"); - return tagText?.textContent || el.textContent; - }); - - // Click on the first tag to edit it - await tags[0].click(); - - // Wait for edit mode - await page.waitForSelector(".tag.editing", { strategy: "pierce" }); - - // Find the edit input and type some text - const editInput = await page.waitForSelector(".tag-input", { - strategy: "pierce", - }); - await editInput.evaluate((el: HTMLInputElement) => { - el.select(); - }); - await editInput.type("should-be-cancelled"); - - // Press Escape to cancel - await page.keyboard.press("Escape"); - await sleep(100); - - // Verify the edit was cancelled and original text is preserved - const updatedTags = await page.$$(".tag", { strategy: "pierce" }); - const currentText = await updatedTags[0].evaluate((el: HTMLElement) => { - const tagText = el.querySelector(".tag-text"); - return tagText?.textContent || el.textContent; - }); - - assertEquals( - currentText?.trim(), - originalText?.trim(), - "Tag text should be unchanged after Escape", - ); - }); - - it("should delete empty tags when backspacing", async () => { - const page = shell.page(); - - console.log("Waiting for component to stabilize..."); - await sleep(500); - - // Helper function to add a tag if needed - const addTag = async (tagText: string) => { - const addTagButton = await page.waitForSelector(".add-tag", { - strategy: "pierce", - }); - - await addTagButton.evaluate((el: HTMLElement) => { - el.click(); - }); - await sleep(100); - - const addInput = await page.waitForSelector(".add-tag-input", { - strategy: "pierce", - }); - await addInput.type(tagText); - await page.keyboard.press("Enter"); - await sleep(100); - }; - - // Get initial count - let initialTags = await page.$$(".tag", { strategy: "pierce" }); - - // Add a tag if none exist - if (initialTags.length === 0) { - await addTag("test-tag"); - initialTags = await page.$$(".tag", { strategy: "pierce" }); - } - - const initialCount = initialTags.length; - - // Click on the first tag to edit it - await initialTags[0].click(); - - // Wait for edit mode - await page.waitForSelector(".tag.editing", { strategy: "pierce" }); - - // Find the edit input and clear all text using keyboard - const editInput = await page.waitForSelector(".tag-input", { - strategy: "pierce", - }); - - // Select all text and delete it - await editInput.evaluate((el: HTMLInputElement) => { - el.select(); - }); - await page.keyboard.press("Delete"); - await sleep(50); // Wait for input to be cleared - - // Press Backspace on empty input - await page.keyboard.press("Backspace"); - await sleep(200); - - // Verify the tag was removed - const remainingTags = await page.$$(".tag", { strategy: "pierce" }); - assertEquals( - remainingTags.length, - initialCount - 1, - "Should have one less tag after backspacing empty tag", - ); - }); -}); + } +} diff --git a/packages/patterns/integration/instantiate-recipe.test.ts b/packages/patterns/integration/instantiate-recipe.test.ts index 1f148596f0..754a351779 100644 --- a/packages/patterns/integration/instantiate-recipe.test.ts +++ b/packages/patterns/integration/instantiate-recipe.test.ts @@ -9,6 +9,9 @@ import { CharmsController } from "@commontools/charm/ops"; const { API_URL, FRONTEND_URL, SPACE_NAME } = env; +// TODO(CT-1101) Need to re-enable these tests to make them more robust +const ignore = true; + describe("instantiate-recipe integration test", () => { const shell = new ShellIntegration(); shell.bindLifecycle(); @@ -41,61 +44,65 @@ describe("instantiate-recipe integration test", () => { if (cc) await cc.dispose(); }); - it("should deploy recipe, click button, and navigate to counter", async () => { - const page = shell.page(); - - await shell.goto({ - frontendUrl: FRONTEND_URL, - view: { - spaceName: SPACE_NAME, - charmId, - }, - identity, - }); - - // Wait for charm to load by waiting for first interactive element - await page.waitForSelector("[data-ct-input]", { strategy: "pierce" }); - - // Store the current URL before any action - const urlBefore = await page.evaluate(() => globalThis.location.href); - console.log("URL before action:", urlBefore); - - const input = await page.waitForSelector("[data-ct-input]", { - strategy: "pierce", - }); - - await input.type("New counter"); - - // Quick wait for input processing - await sleep(100); - - const button = await page.waitForSelector("[data-ct-button]", { - strategy: "pierce", - }); - - await button.click(); - - // Wait for page to soft navigate - await page.waitForFunction((urlBefore) => { - return globalThis.location.href !== urlBefore; - }, { args: [urlBefore] }); - - const urlAfter = await page.evaluate(() => globalThis.location.href); - console.log("URL after clicking:", urlAfter); - - // Verify navigation happened (URL should have changed) - assert( - urlBefore !== urlAfter, - "Should navigate to a new URL after clicking Add button", - ); - - // Verify we're now on a counter page by checking for counter-specific elements - const counterResult = await page.waitForSelector("#counter-result", { - strategy: "pierce", - }); - assert( - counterResult, - "Should find counter-result element after navigation", - ); + it({ + name: "should deploy recipe, click button, and navigate to counter", + ignore, + fn: async () => { + const page = shell.page(); + + await shell.goto({ + frontendUrl: FRONTEND_URL, + view: { + spaceName: SPACE_NAME, + charmId, + }, + identity, + }); + + // Wait for charm to load by waiting for first interactive element + await page.waitForSelector("[data-ct-input]", { strategy: "pierce" }); + + // Store the current URL before any action + const urlBefore = await page.evaluate(() => globalThis.location.href); + console.log("URL before action:", urlBefore); + + const input = await page.waitForSelector("[data-ct-input]", { + strategy: "pierce", + }); + + await input.type("New counter"); + + // Quick wait for input processing + await sleep(100); + + const button = await page.waitForSelector("[data-ct-button]", { + strategy: "pierce", + }); + + await button.click(); + + // Wait for page to soft navigate + await page.waitForFunction((urlBefore) => { + return globalThis.location.href !== urlBefore; + }, { args: [urlBefore] }); + + const urlAfter = await page.evaluate(() => globalThis.location.href); + console.log("URL after clicking:", urlAfter); + + // Verify navigation happened (URL should have changed) + assert( + urlBefore !== urlAfter, + "Should navigate to a new URL after clicking Add button", + ); + + // Verify we're now on a counter page by checking for counter-specific elements + const counterResult = await page.waitForSelector("#counter-result", { + strategy: "pierce", + }); + assert( + counterResult, + "Should find counter-result element after navigation", + ); + }, }); }); diff --git a/packages/patterns/integration/list-operations.test.ts b/packages/patterns/integration/list-operations.test.ts index 9d7d0b5cf8..8e8ccb8217 100644 --- a/packages/patterns/integration/list-operations.test.ts +++ b/packages/patterns/integration/list-operations.test.ts @@ -3,7 +3,7 @@ import { CharmController, CharmsController } from "@commontools/charm/ops"; import { ShellIntegration } from "@commontools/integration/shell-utils"; import { afterAll, beforeAll, describe, it } from "@std/testing/bdd"; import { join } from "@std/path"; -import { assert, assertEquals } from "@std/assert"; +import { assertEquals } from "@std/assert"; import { Identity } from "@commontools/identity"; const { API_URL, FRONTEND_URL, SPACE_NAME } = env; @@ -54,23 +54,27 @@ describe("list-operations simple test", () => { await page.waitForSelector("#main-list", { strategy: "pierce" }); // Click reset to populate with initial data - const resetBtn = await page.$("#reset-demo", { strategy: "pierce" }); - assert(resetBtn, "Should find reset button"); + const resetBtn = await page.waitForSelector("#reset-demo", { + strategy: "pierce", + }); await resetBtn.click(); // Wait for the reset operation to complete by checking the text content - const mainList = await page.$("#main-list", { strategy: "pierce" }); - assert(mainList, "Should find main list element"); - await waitFor(async () => { - const initialText = await mainList!.evaluate((el: HTMLElement) => + const mainList = await page.waitForSelector("#main-list", { + strategy: "pierce", + }); + const initialText = await mainList.evaluate((el: HTMLElement) => el.textContent || "" ); return initialText === "A, B, C, D (4)"; }); // Verify the list populated correctly - const initialText = await mainList!.evaluate((el: HTMLElement) => + const mainList = await page.waitForSelector("#main-list", { + strategy: "pierce", + }); + const initialText = await mainList.evaluate((el: HTMLElement) => el.textContent || "" ); assertEquals( @@ -80,24 +84,26 @@ describe("list-operations simple test", () => { ); // Test delete first item - const deleteFirstBtn = await page.$("#delete-first", { + const deleteFirstBtn = await page.waitForSelector("#delete-first", { strategy: "pierce", }); - await deleteFirstBtn!.click(); + await deleteFirstBtn.click(); // Wait for delete to complete await waitFor(async () => { - const currentMainList = await page.$("#main-list", { + const currentMainList = await page.waitForSelector("#main-list", { strategy: "pierce", }); - const text = await currentMainList!.evaluate((el: HTMLElement) => + const text = await currentMainList.evaluate((el: HTMLElement) => el.textContent || "" ); return text === "B, C, D (3)"; }); - const currentMainList = await page.$("#main-list", { strategy: "pierce" }); - const afterDeleteText = await currentMainList!.evaluate((el: HTMLElement) => + const currentMainList = await page.waitForSelector("#main-list", { + strategy: "pierce", + }); + const afterDeleteText = await currentMainList.evaluate((el: HTMLElement) => el.textContent || "" ); assertEquals( @@ -107,22 +113,26 @@ describe("list-operations simple test", () => { ); // Test insert at start - const insertStartBtn = await page.$("#insert-start", { + const insertStartBtn = await page.waitForSelector("#insert-start", { strategy: "pierce", }); - await insertStartBtn!.click(); + await insertStartBtn.click(); // Wait for insert to complete await waitFor(async () => { - const insertMainList = await page.$("#main-list", { strategy: "pierce" }); - const text = await insertMainList!.evaluate((el: HTMLElement) => + const insertMainList = await page.waitForSelector("#main-list", { + strategy: "pierce", + }); + const text = await insertMainList.evaluate((el: HTMLElement) => el.textContent || "" ); return text === "New Start, B, C, D (4)"; }); - const insertMainList = await page.$("#main-list", { strategy: "pierce" }); - const afterInsertText = await insertMainList!.evaluate((el: HTMLElement) => + const insertMainList = await page.waitForSelector("#main-list", { + strategy: "pierce", + }); + const afterInsertText = await insertMainList.evaluate((el: HTMLElement) => el.textContent || "" ); assertEquals( @@ -132,21 +142,25 @@ describe("list-operations simple test", () => { ); // Test one more operation: delete-last to see if it works - const deleteLastBtn = await page.$("#delete-last", { strategy: "pierce" }); - await deleteLastBtn!.click(); + const deleteLastBtn = await page.waitForSelector("#delete-last", { + strategy: "pierce", + }); + await deleteLastBtn.click(); await waitFor(async () => { - const deleteLastMainList = await page.$("#main-list", { + const deleteLastMainList = await page.waitForSelector("#main-list", { strategy: "pierce", }); - const text = await deleteLastMainList!.evaluate((el: HTMLElement) => + const text = await deleteLastMainList.evaluate((el: HTMLElement) => el.textContent || "" ); return text === "New Start, B, C (3)"; }); - const finalMainList = await page.$("#main-list", { strategy: "pierce" }); - const finalText = await finalMainList!.evaluate((el: HTMLElement) => + const finalMainList = await page.waitForSelector("#main-list", { + strategy: "pierce", + }); + const finalText = await finalMainList.evaluate((el: HTMLElement) => el.textContent || "" ); assertEquals( diff --git a/packages/patterns/integration/nested-counter.test.ts b/packages/patterns/integration/nested-counter.test.ts index b32a519c08..596846a029 100644 --- a/packages/patterns/integration/nested-counter.test.ts +++ b/packages/patterns/integration/nested-counter.test.ts @@ -51,16 +51,15 @@ describe("nested counter integration test", () => { identity, }); - const counterResult = await page.waitForSelector("#counter-result", { - strategy: "pierce", + await waitFor(async () => { + const counterResult = await page.waitForSelector("#counter-result", { + strategy: "pierce", + }); + const initialText = await counterResult.evaluate((el: HTMLElement) => + el.textContent + ); + return initialText?.trim() === "Counter is the 0th number"; }); - assert(counterResult, "Should find counter-result element"); - - // Verify initial value is 0 - const initialText = await counterResult.evaluate((el: HTMLElement) => - el.textContent - ); - assertEquals(initialText?.trim(), "Counter is the 0th number"); // Verify via direct operations that the nested structure works assertEquals(charm.result.get(["value"]), 0); diff --git a/packages/shell/integration/charm.test.ts b/packages/shell/integration/charm.test.ts index 3401b7038a..b9aaea34a9 100644 --- a/packages/shell/integration/charm.test.ts +++ b/packages/shell/integration/charm.test.ts @@ -59,20 +59,25 @@ describe("shell charm tests", () => { identity, }); - let handle = await page.waitForSelector( - "#counter-decrement", - { strategy: "pierce" }, - ); - handle.click(); - await sleep(1000); - handle.click(); - await sleep(1000); - handle = await page.waitForSelector( + // Click twice + for (let i = 0; i < 2; i++) { + const handle = await page.waitForSelector( + "#counter-decrement", + { strategy: "pierce" }, + ); + // Wait for inner text. There's some + // race here where we can click before the + // box model is available. + const _ = await handle.innerText(); + handle.click(); + await sleep(1000); + } + const handle = await page.waitForSelector( "#counter-result", { strategy: "pierce" }, ); - await sleep(1000); - const text = await handle?.innerText(); + await sleep(1500); + const text = await handle.innerText(); assert(text === "Counter is the -2th number"); }); }); diff --git a/packages/shell/integration/iframe-counter-charm.disabled_test.ts b/packages/shell/integration/iframe-counter-charm.disabled_test.ts index e209748aba..3cead41638 100644 --- a/packages/shell/integration/iframe-counter-charm.disabled_test.ts +++ b/packages/shell/integration/iframe-counter-charm.disabled_test.ts @@ -65,7 +65,7 @@ async function getCharmResult(page: Page): Promise<{ count: number }> { // Use the element handle to evaluate in its context return await appView.evaluate((element: XAppView) => { // Access the private _activeCharm property - const activeCharmTask = element._activePatterns; + const activeCharmTask = element._patterns; if (!activeCharmTask) { throw new Error("No _activeCharm property found on element"); @@ -77,6 +77,9 @@ async function getCharmResult(page: Page): Promise<{ count: number }> { // Get the charm controller from the Task's value const charmController = activeCharmTask.value.activePattern; + if (!charmController) { + throw new Error("No active charm controller found"); + } // Get the result from the charm controller const result = charmController.result.get(); diff --git a/packages/shell/src/lib/pattern-factory.ts b/packages/shell/src/lib/pattern-factory.ts index 7e92045f70..28bdc17a46 100644 --- a/packages/shell/src/lib/pattern-factory.ts +++ b/packages/shell/src/lib/pattern-factory.ts @@ -1,8 +1,9 @@ import { CharmController, CharmsController } from "@commontools/charm/ops"; import { HttpProgramResolver } from "@commontools/js-compiler"; import { API_URL } from "./env.ts"; +import { NameSchema } from "@commontools/charm"; -export type BuiltinPatternType = "home" | "space-default"; +export type BuiltinPatternType = "home" | "space-root"; type BuiltinPatternConfig = { url: URL; @@ -16,17 +17,17 @@ const Configs: Record = { url: new URL(`/api/patterns/home.tsx`, API_URL), cause: "home-pattern", }, - "space-default": { + "space-root": { name: "DefaultCharmList", url: new URL(`/api/patterns/default-app.tsx`, API_URL), - cause: "default-charm", + cause: "space-root", }, }; export async function create( cc: CharmsController, type: BuiltinPatternType, -): Promise { +): Promise> { const config = Configs[type]; const manager = cc.manager(); const runtime = manager.runtime; @@ -35,7 +36,11 @@ export async function create( new HttpProgramResolver(config.url.href), ); - const charm = await cc.create(program, { start: true }, config.cause); + const charm = await cc.create( + program, + { start: true }, + config.cause, + ); // Wait for the link to be processed await runtime.idle(); @@ -49,7 +54,7 @@ export async function create( export async function get( cc: CharmsController, -): Promise { +): Promise | undefined> { const pattern = await cc.manager().getDefaultPattern(); if (!pattern) { return undefined; @@ -60,7 +65,7 @@ export async function get( export async function getOrCreate( cc: CharmsController, type: BuiltinPatternType, -): Promise { +): Promise> { const pattern = await get(cc); if (pattern) { return pattern; diff --git a/packages/shell/src/lib/runtime.ts b/packages/shell/src/lib/runtime.ts index 0a9b8a89c1..e2e135813a 100644 --- a/packages/shell/src/lib/runtime.ts +++ b/packages/shell/src/lib/runtime.ts @@ -5,14 +5,20 @@ import { RuntimeTelemetryEvent, RuntimeTelemetryMarkerResult, } from "@commontools/runner"; -import { charmId, CharmManager } from "@commontools/charm"; -import { CharmsController } from "@commontools/charm/ops"; +import { + charmId, + CharmManager, + NameSchema, + nameSchema, +} from "@commontools/charm"; +import { CharmController, CharmsController } from "@commontools/charm/ops"; import { StorageManager } from "@commontools/runner/storage/cache"; import { navigate } from "./navigate.ts"; import * as Inspector from "@commontools/runner/storage/inspector"; import { setupIframe } from "./iframe-ctx.ts"; import { getLogger } from "@commontools/utils/logger"; import { AppView } from "./app/view.ts"; +import * as PatternFactory from "./pattern-factory.ts"; const logger = getLogger("shell.telemetry", { enabled: false, @@ -34,15 +40,20 @@ export class RuntimeInternals extends EventTarget { #inspector: Inspector.Channel; #disposed = false; #space: string; // The MemorySpace DID + #spaceRootPattern?: CharmController; + #spaceRootPatternType: PatternFactory.BuiltinPatternType; + #patternCache: Map> = new Map(); private constructor( cc: CharmsController, telemetry: RuntimeTelemetry, space: string, + spaceRootPatternType: PatternFactory.BuiltinPatternType, ) { super(); this.#cc = cc; this.#space = space; + this.#spaceRootPatternType = spaceRootPatternType; const runtimeId = this.#cc.manager().runtime.id; this.#inspector = new Inspector.Channel( runtimeId, @@ -69,6 +80,45 @@ export class RuntimeInternals extends EventTarget { return this.#space; } + // Returns the space root pattern, creating it if it doesn't exist. + // The space root pattern type is determined at RuntimeInternals creation + // based on the view type (home vs space). + async getSpaceRootPattern(): Promise> { + this.#check(); + if (this.#spaceRootPattern) { + return this.#spaceRootPattern; + } + + const pattern = await PatternFactory.getOrCreate( + this.#cc, + this.#spaceRootPatternType, + ); + + // Track as recently accessed + await this.#cc.manager().trackRecentCharm(pattern.getCell()); + + this.#patternCache.set(pattern.id, pattern); + return pattern; + } + + // Returns a pattern by ID, starting it if requested. + // Patterns are cached by ID. + async getPattern(id: string): Promise> { + this.#check(); + const cached = this.#patternCache.get(id); + if (cached) { + return cached; + } + + const pattern = await this.#cc.get(id, true, nameSchema); + + // Track as recently accessed + await this.#cc.manager().trackRecentCharm(pattern.getCell()); + + this.#patternCache.set(id, pattern); + return pattern; + } + async dispose() { if (this.#disposed) return; this.#disposed = true; @@ -106,20 +156,24 @@ export class RuntimeInternals extends EventTarget { ): Promise { let session; let spaceName; + let spaceRootPatternType: PatternFactory.BuiltinPatternType; if ("builtin" in view) { switch (view.builtin) { case "home": session = await createSession({ identity, spaceDid: identity.did() }); spaceName = ""; + spaceRootPatternType = "home"; break; } } else if ("spaceName" in view) { session = await createSession({ identity, spaceName: view.spaceName }); spaceName = view.spaceName; + spaceRootPatternType = "space-root"; } else if ("spaceDid" in view) { session = await createSession({ identity, spaceDid: view.spaceDid }); + spaceRootPatternType = "space-root"; } - if (!session) { + if (!session || !spaceRootPatternType!) { throw new Error("Unexpected view provided."); } @@ -234,6 +288,11 @@ export class RuntimeInternals extends EventTarget { charmManager = new CharmManager(session, runtime); await charmManager.synced(); const cc = new CharmsController(charmManager); - return new RuntimeInternals(cc, telemetry, session.space); + return new RuntimeInternals( + cc, + telemetry, + session.space, + spaceRootPatternType, + ); } } diff --git a/packages/shell/src/views/AppView.ts b/packages/shell/src/views/AppView.ts index a29d5e00fa..39dfd90a2b 100644 --- a/packages/shell/src/views/AppView.ts +++ b/packages/shell/src/views/AppView.ts @@ -1,20 +1,17 @@ import { css, html } from "lit"; import { property, state } from "lit/decorators.js"; - -import { AppView } from "../lib/app/mod.ts"; import { BaseView, createDefaultAppState } from "./BaseView.ts"; import { KeyStore } from "@commontools/identity"; import { RuntimeInternals } from "../lib/runtime.ts"; import { DebuggerController } from "../lib/debugger-controller.ts"; import "./DebuggerView.ts"; -import { Task } from "@lit/task"; -import { CharmController, CharmsController } from "@commontools/charm/ops"; +import { Task, TaskStatus } from "@lit/task"; +import { CharmController } from "@commontools/charm/ops"; import { CellEventTarget, CellUpdateEvent } from "../lib/cell-event-target.ts"; import { NAME } from "@commontools/runner"; -import { type NameSchema, nameSchema } from "@commontools/charm"; +import { type NameSchema } from "@commontools/charm"; import { updatePageTitle } from "../lib/navigate.ts"; import { KeyboardController } from "../lib/keyboard-router.ts"; -import * as PatternFactory from "../lib/pattern-factory.ts"; export class XAppView extends BaseView { static override styles = css` @@ -52,7 +49,7 @@ export class XAppView extends BaseView { @property({ attribute: false }) keyStore?: KeyStore; - @property({ attribute: false }) + @state() charmTitle?: string; @property({ attribute: false }) @@ -85,45 +82,80 @@ export class XAppView extends BaseView { this.hasSidebarContent = event.detail.hasSidebarContent; }; - // Do not make private, integration tests access this directly. - // - // This fetches the active pattern and space default pattern derived - // from the current view. - _activePatterns = new Task(this, { + // Fetches the space root pattern from the space. + _spaceRootPattern = new Task(this, { task: async ( - [app, rt], + [rt], ): Promise< - | { activePattern: CharmController; defaultPattern: CharmController } + | CharmController | undefined > => { - if (!rt) { - this.#setTitleSubscription(); - return; - } + if (!rt) return; + return await rt.getSpaceRootPattern(); + }, + args: () => [this.rt], + }); - const patterns = await viewToPatterns( - rt.cc(), - app.view, - this._activePatterns.value?.activePattern, - ); - if (!patterns) { - this.#setTitleSubscription(); - return; + // This fetches the selected pattern, the explicitly chosen pattern + // to render via URL e.g. `/space/someCharmId`. + _selectedPattern = new Task(this, { + task: async ( + [app, rt], + ): Promise< + | CharmController + | undefined + > => { + if (!rt) return; + if ("charmId" in app.view && app.view.charmId) { + return await rt.getPattern(app.view.charmId); } - - // Record the charm as recently accessed so recents stay fresh. - await rt.cc().manager().trackRecentCharm( - patterns.activePattern.getCell(), - ); - this.#setTitleSubscription( - patterns.activePattern as CharmController, - ); - - return patterns; }, args: () => [this.app, this.rt], }); + // This derives a space root pattern as well as an "active" (main) + // pattern for use in child views. + // This hybrid task intentionally only uses completed/fresh + // source patterns to avoid unsyncing state. + _patterns = new Task(this, { + task: function ( + [ + app, + spaceRootPatternValue, + spaceRootPatternStatus, + selectedPatternValue, + selectedPatternStatus, + ], + ): { + activePattern: CharmController | undefined; + spaceRootPattern: CharmController | undefined; + } { + const spaceRootPattern = spaceRootPatternStatus === TaskStatus.COMPLETE + ? spaceRootPatternValue + : undefined; + // The "active" pattern is the main pattern to be rendered. + // This may be the same as the space root pattern, unless we're + // in a view that specifies a different pattern to use. + const useSpaceRootAsActive = !("charmId" in app.view && app.view.charmId); + const activePattern = useSpaceRootAsActive + ? spaceRootPattern + : selectedPatternStatus === TaskStatus.COMPLETE + ? selectedPatternValue + : undefined; + return { + activePattern, + spaceRootPattern, + }; + }, + args: () => [ + this.app, + this._spaceRootPattern.value, + this._spaceRootPattern.status, + this._selectedPattern.value, + this._selectedPattern.status, + ], + }); + #setTitleSubscription(activeCharm?: CharmController) { if (!activeCharm) { if (this.titleSubscription) { @@ -174,31 +206,42 @@ export class XAppView extends BaseView { } // Update debugger visibility from app state - if (changedProperties.has("app") && this.app) { + if (changedProperties.has("app")) { this.debuggerController.setVisibility( this.app.config.showDebuggerView ?? false, ); } } + // Always defer to the loaded active pattern for the ID, + // but until that loads, use an ID in the view if available. + private getActivePatternId(): string | undefined { + const activePattern = this._patterns.value?.activePattern; + if (activePattern?.id) return activePattern.id; + if ("charmId" in this.app.view && this.app.view.charmId) { + return this.app.view.charmId; + } + } + override render() { const config = this.app.config ?? {}; - const unauthenticated = html` - - `; - const patterns = this._activePatterns.value; - const activePattern = patterns?.activePattern; - const defaultPattern = patterns?.defaultPattern; + const { activePattern, spaceRootPattern } = this._patterns.value ?? {}; + this.#setTitleSubscription(activePattern); + const authenticated = html` `; + const unauthenticated = html` + + `; + const charmId = this.getActivePatternId(); const spaceName = this.app && "spaceName" in this.app.view ? this.app.view.spaceName : undefined; @@ -211,7 +254,7 @@ export class XAppView extends BaseView { .rt="${this.rt}" .keyStore="${this.keyStore}" .charmTitle="${this.charmTitle}" - .charmId="${activePattern?.id}" + .charmId="${charmId}" .showShellCharmListView="${config.showShellCharmListView ?? false}" .showDebuggerView="${config.showDebuggerView ?? false}" .showSidebar="${config.showSidebar ?? false}" @@ -221,7 +264,7 @@ export class XAppView extends BaseView { ${content} - ${this.app?.identity + ${this.app.identity ? html` , -): Promise< - { - activePattern: CharmController; - defaultPattern: CharmController; - } | undefined -> { - if ("builtin" in view) { - if (view.builtin !== "home") { - console.warn("Unsupported view type"); - return; - } - const pattern = await PatternFactory.getOrCreate(cc, "home"); - return { activePattern: pattern, defaultPattern: pattern }; - } else if ("spaceDid" in view) { - console.warn("Unsupported view type"); - return; - } else if ("spaceName" in view) { - const defaultPattern = await PatternFactory.getOrCreate( - cc, - "space-default", - ); - - let activePattern: CharmController | undefined; - // If viewing a specific charm, use it as active; otherwise use default - if (view.charmId) { - if (currentActive && currentActive.id === view.charmId) { - activePattern = currentActive; - } else { - activePattern = await cc.get( - view.charmId, - true, - nameSchema, - ); - } - } else { - activePattern = defaultPattern; - } - return { activePattern, defaultPattern }; - } -} - globalThis.customElements.define("x-app-view", XAppView); diff --git a/packages/shell/src/views/BodyView.ts b/packages/shell/src/views/BodyView.ts index 8b961247f7..f3f596a09a 100644 --- a/packages/shell/src/views/BodyView.ts +++ b/packages/shell/src/views/BodyView.ts @@ -45,10 +45,10 @@ export class XBodyView extends BaseView { rt?: RuntimeInternals; @property({ attribute: false }) - activeCharm?: CharmController; + activePattern?: CharmController; @property({ attribute: false }) - defaultCharm?: CharmController; + spaceRootPattern?: CharmController; @property() showShellCharmListView = false; @@ -96,16 +96,16 @@ export class XBodyView extends BaseView { `; } - const mainContent = this.activeCharm + const mainContent = this.activePattern ? html` - - + + ` : null; - const sidebarCell = this.activeCharm?.getCell().key("sidebarUI"); - const fabCell = this.defaultCharm?.getCell().key("fabUI"); + const sidebarCell = this.activePattern?.getCell().key("sidebarUI"); + const fabCell = this.spaceRootPattern?.getCell().key("fabUI"); // Update sidebar content detection // TODO(seefeld): Fix possible race here where charm is already set, but diff --git a/packages/shell/src/views/RootView.ts b/packages/shell/src/views/RootView.ts index 363198ddb7..19cdebdd80 100644 --- a/packages/shell/src/views/RootView.ts +++ b/packages/shell/src/views/RootView.ts @@ -17,9 +17,10 @@ import { runtimeContext, spaceContext } from "@commontools/ui"; import { provide } from "@lit/context"; // The root element for the shell application. -// Handles processing `Command`s from children elements, -// updating the `AppState`, and providing changes -// to children elements. +// +// Derives `RuntimeInternals` for the application from its `AppState`. +// `Command` mutates the app state, which can be fired as events +// from children elements. export class XRootView extends BaseView { static override styles = css` :host { From 5f7f1067c46798624b922196db24d0b16c23ba30 Mon Sep 17 00:00:00 2001 From: Jordan Santell Date: Fri, 5 Dec 2025 13:00:45 -0800 Subject: [PATCH 3/3] Revert 6e979d0b0c021877882e8f3c13262073bcf36103 (#2208) Revert "chore: shell: Separate the derivation of active pattern and space root pattern, syncing state (#2188)" This reverts commit 6e979d0b0c021877882e8f3c13262073bcf36103. --- packages/charm/src/ops/charms-controller.ts | 8 +- packages/patterns/integration/counter.test.ts | 50 +- .../patterns/integration/ct-checkbox.test.ts | 79 ++- packages/patterns/integration/ct-list.test.ts | 364 ++++++++----- .../patterns/integration/ct-render.test.ts | 37 +- packages/patterns/integration/ct-tags.test.ts | 485 ++++++++++++------ .../integration/instantiate-recipe.test.ts | 119 ++--- .../integration/list-operations.test.ts | 66 +-- .../integration/nested-counter.test.ts | 17 +- packages/shell/integration/charm.test.ts | 27 +- .../iframe-counter-charm.disabled_test.ts | 5 +- packages/shell/src/lib/pattern-factory.ts | 19 +- packages/shell/src/lib/runtime.ts | 67 +-- packages/shell/src/views/AppView.ts | 182 +++---- packages/shell/src/views/BodyView.ts | 14 +- packages/shell/src/views/RootView.ts | 7 +- 16 files changed, 887 insertions(+), 659 deletions(-) diff --git a/packages/charm/src/ops/charms-controller.ts b/packages/charm/src/ops/charms-controller.ts index 84ab85f44e..56eecb7a2a 100644 --- a/packages/charm/src/ops/charms-controller.ts +++ b/packages/charm/src/ops/charms-controller.ts @@ -29,14 +29,14 @@ export class CharmsController { return this.#manager; } - async create( + async create( program: RuntimeProgram | string, options: CreateCharmOptions = {}, cause: string | undefined = undefined, - ): Promise> { + ): Promise> { this.disposeCheck(); const recipe = await compileProgram(this.#manager, program); - const charm = await this.#manager.runPersistent( + const charm = await this.#manager.runPersistent( recipe, options.input, cause, @@ -45,7 +45,7 @@ export class CharmsController { ); await this.#manager.runtime.idle(); await this.#manager.synced(); - return new CharmController(this.#manager, charm); + return new CharmController(this.#manager, charm); } async get( diff --git a/packages/patterns/integration/counter.test.ts b/packages/patterns/integration/counter.test.ts index e18b61d52d..1a0e55311f 100644 --- a/packages/patterns/integration/counter.test.ts +++ b/packages/patterns/integration/counter.test.ts @@ -3,7 +3,7 @@ import { CharmController, CharmsController } from "@commontools/charm/ops"; import { ShellIntegration } from "@commontools/integration/shell-utils"; import { afterAll, beforeAll, describe, it } from "@std/testing/bdd"; import { join } from "@std/path"; -import { assertEquals } from "@std/assert"; +import { assert, assertEquals } from "@std/assert"; import { Identity } from "@commontools/identity"; import { FileSystemProgramResolver } from "@commontools/js-compiler"; @@ -50,16 +50,16 @@ describe("counter direct operations test", () => { identity, }); - // Verify initial value is 0 - await waitFor(async () => { - const counterResult = await page.waitForSelector("#counter-result", { - strategy: "pierce", - }); - const initialText = await counterResult.evaluate((el: HTMLElement) => - el.textContent - ); - return initialText?.trim() === "Counter is the 0th number"; + const counterResult = await page.waitForSelector("#counter-result", { + strategy: "pierce", }); + assert(counterResult, "Should find counter-result element"); + + // Verify initial value is 0 + const initialText = await counterResult.evaluate((el: HTMLElement) => + el.textContent + ); + assertEquals(initialText?.trim(), "Counter is the 0th number"); assertEquals(charm.result.get(["value"]), 0); }); @@ -67,17 +67,15 @@ describe("counter direct operations test", () => { it("should update counter value via direct operation (live)", async () => { const page = shell.page(); - await page.waitForSelector("#counter-result", { + // Get the counter result element + const counterResult = await page.waitForSelector("#counter-result", { strategy: "pierce", }); + console.log("Setting counter value to 42 via direct operation"); await charm.result.set(42, ["value"]); await waitFor(async () => { - const counterResult = await page.waitForSelector("#counter-result", { - strategy: "pierce", - }); - const updatedText = await counterResult.evaluate((el: HTMLElement) => el.textContent ); @@ -107,15 +105,19 @@ describe("counter direct operations test", () => { }); // Get the counter result element after refresh - await waitFor(async () => { - const counterResult = await page.waitForSelector("#counter-result", { - strategy: "pierce", - }); - - const textAfterRefresh = await counterResult.evaluate((el: HTMLElement) => - el.textContent - ); - return textAfterRefresh?.trim() === "Counter is the 42th number"; + const counterResult = await page.waitForSelector("#counter-result", { + strategy: "pierce", }); + assert(counterResult, "Should find counter-result element after refresh"); + + // Check if the UI shows the updated value after refresh + const textAfterRefresh = await counterResult.evaluate((el: HTMLElement) => + el.textContent + ); + assertEquals( + textAfterRefresh?.trim(), + "Counter is the 42th number", + "UI should show persisted value after refresh", + ); }); }); diff --git a/packages/patterns/integration/ct-checkbox.test.ts b/packages/patterns/integration/ct-checkbox.test.ts index 759388039d..24d0071e8c 100644 --- a/packages/patterns/integration/ct-checkbox.test.ts +++ b/packages/patterns/integration/ct-checkbox.test.ts @@ -1,7 +1,9 @@ -import { env, waitFor } from "@commontools/integration"; +import { env } from "@commontools/integration"; +import { sleep } from "@commontools/utils/sleep"; import { ShellIntegration } from "@commontools/integration/shell-utils"; import { afterAll, beforeAll, describe, it } from "@std/testing/bdd"; import { join } from "@std/path"; +import { assertEquals } from "@std/assert"; import { Identity } from "@commontools/identity"; import { CharmsController } from "@commontools/charm/ops"; import { ANYONE_USER } from "@commontools/memory/acl"; @@ -65,62 +67,49 @@ testComponents.forEach(({ name, file }) => { it("should show disabled content initially", async () => { const page = shell.page(); - await waitFor(async () => { - const featureStatus = await page.waitForSelector("#feature-status", { - strategy: "pierce", - }); - const statusText = await featureStatus.evaluate((el: HTMLElement) => - el.textContent - ); - return statusText?.trim() === "⚠ Feature is disabled"; + const featureStatus = await page.waitForSelector("#feature-status", { + strategy: "pierce", }); + const statusText = await featureStatus.evaluate((el: HTMLElement) => + el.textContent + ); + assertEquals(statusText?.trim(), "⚠ Feature is disabled"); }); it("should toggle to enabled content when checkbox is clicked", async () => { const page = shell.page(); - await waitFor(async () => { - const checkbox = await page.waitForSelector("ct-checkbox", { - strategy: "pierce", - }); - // This could throw due to lacking a box model to click on. - // Catch in lieu of handling time sensitivity. - try { - await checkbox.click(); - } catch (_) { - return false; - } - const featureStatus = await page.waitForSelector("#feature-status", { - strategy: "pierce", - }); - const statusText = await featureStatus.evaluate((el: HTMLElement) => - el.textContent - ); - return statusText?.trim() === "✓ Feature is enabled!"; + const checkbox = await page.waitForSelector("ct-checkbox", { + strategy: "pierce", + }); + await checkbox.click(); + await sleep(500); + + const featureStatus = await page.$("#feature-status", { + strategy: "pierce", }); + const statusText = await featureStatus?.evaluate((el: HTMLElement) => + el.textContent + ); + assertEquals(statusText?.trim(), "✓ Feature is enabled!"); }); it("should toggle back to disabled content when checkbox is clicked again", async () => { const page = shell.page(); - await waitFor(async () => { - const checkbox = await page.waitForSelector("ct-checkbox", { - strategy: "pierce", - }); - // This could throw due to lacking a box model to click on. - // Catch in lieu of handling time sensitivity. - try { - await checkbox.click(); - } catch (_) { - return false; - } - const featureStatus = await page.waitForSelector("#feature-status", { - strategy: "pierce", - }); - const statusText = await featureStatus.evaluate((el: HTMLElement) => - el.textContent - ); - return statusText?.trim() === "⚠ Feature is disabled"; + + const checkbox = await page.$("ct-checkbox", { + strategy: "pierce", + }); + await checkbox?.click(); + await sleep(1000); + + const featureStatus = await page.$("#feature-status", { + strategy: "pierce", }); + const statusText = await featureStatus?.evaluate((el: HTMLElement) => + el.textContent + ); + assertEquals(statusText?.trim(), "⚠ Feature is disabled"); }); }); }); diff --git a/packages/patterns/integration/ct-list.test.ts b/packages/patterns/integration/ct-list.test.ts index 45556d7c40..6a9f5d6103 100644 --- a/packages/patterns/integration/ct-list.test.ts +++ b/packages/patterns/integration/ct-list.test.ts @@ -1,11 +1,11 @@ -import { env, Page, waitFor } from "@commontools/integration"; +import { env } from "@commontools/integration"; +import { sleep } from "@commontools/utils/sleep"; import { ShellIntegration } from "@commontools/integration/shell-utils"; import { afterAll, beforeAll, describe, it } from "@std/testing/bdd"; import { join } from "@std/path"; import { assert, assertEquals } from "@std/assert"; import { Identity } from "@commontools/identity"; import { CharmsController } from "@commontools/charm/ops"; -import { ElementHandle } from "@astral/astral"; const { API_URL, FRONTEND_URL, SPACE_NAME } = env; @@ -56,30 +56,102 @@ describe("ct-list integration test", () => { it("should add items to the list", async () => { const page = shell.page(); - const list = new List(page); - await list.addItem("First item"); - await list.waitForItemCount(1); + // Find the add item input in ct-list + const addInput = await page.waitForSelector(".add-item-input", { + strategy: "pierce", + }); + + // Add first item + await addInput.click(); + await addInput.type("First item"); + await page.keyboard.press("Enter"); + await sleep(50); // Quick wait for DOM update + + // Add second item - the input should be cleared automatically + await addInput.type("Second item"); + await page.keyboard.press("Enter"); + await sleep(50); // Quick wait for DOM update + + // Add third item + await addInput.type("Third item"); + await page.keyboard.press("Enter"); + await sleep(50); // Quick wait for DOM update + + // Verify items were added + const listItems = await page.$$(".list-item", { strategy: "pierce" }); + assertEquals(listItems.length, 3, "Should have 3 items in the list"); + + // Debug: Log the structure of list items + console.log("List item structure:"); + for (let i = 0; i < listItems.length; i++) { + const itemInfo = await listItems[i].evaluate( + (el: HTMLElement, idx: number) => { + const buttons = el.querySelectorAll("button"); + return { + index: idx, + className: el.className, + innerText: el.innerText, + buttonCount: buttons.length, + buttons: Array.from(buttons).map((b) => ({ + className: b.className, + title: b.title || "no title", + innerText: b.innerText, + })), + }; + }, + { args: [i] } as any, + ); + console.log(`Item ${i}:`, itemInfo); + } - await list.addItem("Second item"); - await list.waitForItemCount(2); + // Quick wait for content to render + await sleep(100); + + // Verify item content + const firstItemText = await listItems[0].evaluate((el: HTMLElement) => { + const content = el.querySelector(".item-content") || + el.querySelector("div.item-content"); + return content?.textContent || el.textContent; + }); + assertEquals(firstItemText?.trim(), "First item"); - await list.addItem("Third item"); - await list.waitForItemCount(3); + const secondItemText = await listItems[1].evaluate((el: HTMLElement) => { + const content = el.querySelector(".item-content") || + el.querySelector("div.item-content"); + return content?.textContent || el.textContent; + }); + assertEquals(secondItemText?.trim(), "Second item"); - assertEquals(await list.getItemsText(), [ - "First item", - "Second item", - "Third item", - ]); + const thirdItemText = await listItems[2].evaluate((el: HTMLElement) => { + const content = el.querySelector(".item-content") || + el.querySelector("div.item-content"); + return content?.textContent || el.textContent; + }); + assertEquals(thirdItemText?.trim(), "Third item"); }); it("should update the list title", async () => { const page = shell.page(); - const list = new List(page); - await list.setTitle("My Shopping List"); - assertEquals(await list.getTitle(), "My Shopping List"); + // Find the title input + const titleInput = await page.$("input[placeholder='List title']", { + strategy: "pierce", + }); + assert(titleInput, "Should find title input"); + + // Clear the existing text first + await titleInput.click(); + await titleInput.evaluate((el: HTMLInputElement) => { + el.select(); // Select all text + }); + await titleInput.type("My Shopping List"); + + // Verify title was updated (no wait needed for input value) + const titleValue = await titleInput.evaluate((el: HTMLInputElement) => + el.value + ); + assertEquals(titleValue, "My Shopping List"); }); // TODO(#CT-703): Fix this test - there's a bug where programmatic clicks on the remove button @@ -88,101 +160,142 @@ describe("ct-list integration test", () => { // versus real user clicks. it("should remove items from the list", async () => { const page = shell.page(); - const list = new List(page); - const items = await list.getItems(); - assert(items.length > 0, "Should have items to remove"); - const initialCount = items.length; + console.log("Waiting for component to stabilize..."); + await sleep(500); - await list.removeItem(items[0]); - await list.waitForItemCount(initialCount - 1); + // Get initial count + const initialItems = await page.$$(".list-item", { strategy: "pierce" }); + const initialCount = initialItems.length; + console.log(`Initial item count: ${initialCount}`); + assert(initialCount > 0, "Should have items to remove"); - assertEquals(await list.getItemsText(), [ - "Second item", - "Third item", - ]); - }); + // Find and click the first remove button + const removeButtons = await page.$$("button.item-action.remove", { + strategy: "pierce", + }); + console.log(`Found ${removeButtons.length} remove buttons`); + assert(removeButtons.length > 0, "Should find remove buttons"); + + // Debug: check what we're about to click + const buttonText = await removeButtons[0].evaluate((el: HTMLElement) => { + return { + className: el.className, + title: el.title, + innerText: el.innerText, + parentText: el.parentElement?.innerText || "no parent", + }; + }); + console.log("About to click button:", buttonText); - it("should edit items in the list", async () => { - const page = shell.page(); - const list = new List(page); + // Try clicking more carefully + console.log("Waiting before click..."); + await sleep(100); + + // Alternative approach: dispatch click event + await removeButtons[0].evaluate((button: HTMLElement) => { + console.log("About to dispatch click event on button:", button); + button.dispatchEvent( + new MouseEvent("click", { + bubbles: true, + cancelable: true, + view: window, + }), + ); + }); + console.log("Dispatched click event on first remove button"); - const items = await list.getItems(); - assert(items.length > 0, "Should have items to edit"); + // Check immediately after click + await sleep(50); + const immediateItems = await page.$$(".list-item", { strategy: "pierce" }); + console.log( + `Immediately after click, found ${immediateItems.length} items`, + ); + + // Wait for DOM to update after removal using Astral's waitForFunction + await page.waitForFunction((expectedCount) => { + const items = document.querySelectorAll(".list-item"); + return items.length !== expectedCount; + }, { args: [initialCount] }); + + // Verify item was removed - try multiple times + let remainingItems = await page.$$(".list-item", { strategy: "pierce" }); + console.log(`After removal, found ${remainingItems.length} items`); + + // If still showing same count, wait a bit more and try again + if (remainingItems.length === initialCount) { + console.log("DOM not updated yet, waiting more..."); + await sleep(500); + remainingItems = await page.$$(".list-item", { strategy: "pierce" }); + console.log( + `After additional wait, found ${remainingItems.length} items`, + ); + } - const newText = "Edited Second Item"; - await list.editItem(items[0], newText); - await waitFor(() => - list.getItems().then((els) => list.getItemText(els[0])).then((text) => - text === newText - ) + assertEquals( + remainingItems.length, + initialCount - 1, + "Should have one less item after removal", ); - assertEquals(await list.getItemsText(), [ - "Edited Second Item", - "Third item", - ]); + // Verify the first item is now what was the second item + if (remainingItems.length > 0) { + const firstRemainingText = await remainingItems[0].evaluate( + (el: HTMLElement) => { + const content = el.querySelector(".item-content") || + el.querySelector("div.item-content"); + return content?.textContent || el.textContent; + }, + ); + assertEquals( + firstRemainingText?.trim(), + "Second item", + "First item should now be the second item", + ); + } }); -}); -class List { - #page: Page; - constructor(page: Page) { - this.#page = page; - } + it("should edit items in the list", async () => { + const page = shell.page(); - getItems(): Promise { - return this.#page.$$(".list-item", { strategy: "pierce" }); - } + console.log("Waiting for component to stabilize..."); + await sleep(500); - async getItemsText(): Promise> { - const elements = await this.getItems(); - return Promise.all(elements.map((el) => this.getItemText(el))); - } + // Get initial items + const initialItems = await page.$$(".list-item", { strategy: "pierce" }); + const initialCount = initialItems.length; + console.log(`Initial item count: ${initialCount}`); + assert(initialCount > 0, "Should have items to edit"); - async getItemText(element: ElementHandle): Promise { - return await element.evaluate((el: HTMLElement) => { + // Get the initial text of the first item + const initialText = await initialItems[0].evaluate((el: HTMLElement) => { const content = el.querySelector(".item-content") || el.querySelector("div.item-content"); - return (content?.textContent || el.textContent)?.trim(); + return content?.textContent || el.textContent; }); - } - - async waitForItemCount(expected: number): Promise { - await waitFor(() => this.getItems().then((els) => els.length === expected)); - } + console.log(`Initial text of first item: "${initialText?.trim()}"`); - async addItem(text: string): Promise { - const addInput = await this.#page.waitForSelector(".add-item-input", { + // Find and click the first edit button + const editButtons = await page.$$("button.item-action.edit", { strategy: "pierce", }); - await addInput.click(); - await addInput.type(text); - await this.#page.keyboard.press("Enter"); - } - - async removeItem(element: ElementHandle): Promise { - // Find the remove button within this item - const removeButton = await element.$("button.item-action.remove"); - assert(removeButton, "Should find remove button in item"); - - await removeButton.evaluate((button: HTMLElement) => { - button.dispatchEvent( - new MouseEvent("click", { - bubbles: true, - cancelable: true, - view: window, - }), - ); + console.log(`Found ${editButtons.length} edit buttons`); + assert(editButtons.length > 0, "Should find edit buttons"); + + // Debug: check what we're about to click + const buttonText = await editButtons[0].evaluate((el: HTMLElement) => { + return { + className: el.className, + title: el.title, + innerText: el.innerText, + parentText: el.parentElement?.innerText || "no parent", + }; }); - } + console.log("About to click edit button:", buttonText); - async editItem(element: ElementHandle, newText: string): Promise { - // Find the edit button within this item - const editButton = await element.$("button.item-action.edit"); - assert(editButton, "Should find edit button in item"); - - await editButton.evaluate((button: HTMLElement) => { + // Click the edit button to enter edit mode + await editButtons[0].evaluate((button: HTMLElement) => { + console.log("About to dispatch click event on edit button:", button); button.dispatchEvent( new MouseEvent("click", { bubbles: true, @@ -191,40 +304,59 @@ class List { }), ); }); + console.log("Dispatched click event on first edit button"); - // Wait for edit mode to activate - await this.#page.waitForSelector(".list-item.editing", { - strategy: "pierce", - }); + // Wait for edit mode to activate and look for the editing state + await page.waitForSelector(".list-item.editing", { strategy: "pierce" }); + console.log("Edit mode activated - found .list-item.editing"); - // Find the edit input and type new text - const editInput = await this.#page.waitForSelector(".edit-input", { + // Look for the specific edit input field that appears only during editing + const editInput = await page.$(".edit-input", { strategy: "pierce", }); + assert(editInput, "Should find .edit-input field during editing"); + + // Verify the input is focused (it should have autofocus) + const isFocused = await editInput.evaluate((el: HTMLInputElement) => + document.activeElement === el + ); + console.log(`Edit input is focused: ${isFocused}`); + + // Clear the existing text and type new text await editInput.evaluate((el: HTMLInputElement) => { - el.select(); + el.select(); // Select all text }); + const newText = "Edited First Item"; await editInput.type(newText); - await this.#page.keyboard.press("Enter"); - } + console.log(`Typed new text: "${newText}"`); - async setTitle(title: string): Promise { - const titleInput = await this.#page.waitForSelector( - "input[placeholder='List title']", - { strategy: "pierce" }, + // Press Enter to confirm the edit + await page.keyboard.press("Enter"); + console.log("Pressed Enter to confirm edit"); + + // Wait for the edit to be processed + await sleep(200); + + // Verify the item was edited + const updatedItems = await page.$$(".list-item", { strategy: "pierce" }); + assertEquals( + updatedItems.length, + initialCount, + "Should have same number of items after edit", ); - await titleInput.click(); - await titleInput.evaluate((el: HTMLInputElement) => { - el.select(); + + // Check that the first item's text has been updated + const updatedText = await updatedItems[0].evaluate((el: HTMLElement) => { + const content = el.querySelector(".item-content") || + el.querySelector("div.item-content"); + return content?.textContent || el.textContent; }); - await titleInput.type(title); - } + console.log(`Updated text of first item: "${updatedText?.trim()}"`); - async getTitle(): Promise { - const titleInput = await this.#page.waitForSelector( - "input[placeholder='List title']", - { strategy: "pierce" }, + assertEquals( + updatedText?.trim(), + newText, + "First item should have updated text", ); - return await titleInput.evaluate((el: HTMLInputElement) => el.value); - } -} + }); +}); diff --git a/packages/patterns/integration/ct-render.test.ts b/packages/patterns/integration/ct-render.test.ts index 56aad22a6a..d5545d45cf 100644 --- a/packages/patterns/integration/ct-render.test.ts +++ b/packages/patterns/integration/ct-render.test.ts @@ -51,15 +51,16 @@ describe("ct-render integration test", () => { identity, }); - await waitFor(async () => { - const counterResult = await page.waitForSelector("#counter-result", { - strategy: "pierce", - }); - const initialText = await counterResult.evaluate((el: HTMLElement) => - el.textContent - ); - return initialText?.trim() === "Counter is the 0th number"; + const counterResult = await page.waitForSelector("#counter-result", { + strategy: "pierce", }); + assert(counterResult, "Should find counter-result element"); + + // Verify initial value is 0 + const initialText = await counterResult.evaluate((el: HTMLElement) => + el.textContent + ); + assertEquals(initialText?.trim(), "Counter is the 0th number"); // Verify via direct operations that the ct-render structure works const value = charm.result.get(["value"]); @@ -106,16 +107,18 @@ describe("ct-render integration test", () => { identity, }); - await waitFor(async () => { - const counterResult = await page.waitForSelector("#counter-result", { - strategy: "pierce", - }); - const textAfterUpdate = await counterResult.evaluate((el: HTMLElement) => - el.textContent - ); - return textAfterUpdate?.trim() === - "Counter is the 5th number"; + // Check if the UI shows the updated value + const counterResult = await page.waitForSelector("#counter-result", { + strategy: "pierce", }); + const textAfterUpdate = await counterResult.evaluate((el: HTMLElement) => + el.textContent + ); + assertEquals( + textAfterUpdate?.trim(), + "Counter is the 5th number", + "UI should show updated value from direct operation", + ); }); it("should verify only ONE counter display", async () => { diff --git a/packages/patterns/integration/ct-tags.test.ts b/packages/patterns/integration/ct-tags.test.ts index 19ae0ac968..82c7b90e31 100644 --- a/packages/patterns/integration/ct-tags.test.ts +++ b/packages/patterns/integration/ct-tags.test.ts @@ -1,12 +1,11 @@ -import { env, Page, waitFor } from "@commontools/integration"; +import { env } from "@commontools/integration"; import { sleep } from "@commontools/utils/sleep"; import { ShellIntegration } from "@commontools/integration/shell-utils"; import { afterAll, beforeAll, describe, it } from "@std/testing/bdd"; import { join } from "@std/path"; -import { assertEquals } from "@std/assert"; +import { assert, assertEquals } from "@std/assert"; import { Identity } from "@commontools/identity"; import { CharmsController } from "@commontools/charm/ops"; -import { ElementHandle } from "@astral/astral"; const { API_URL, FRONTEND_URL, SPACE_NAME } = env; @@ -57,188 +56,212 @@ describe("ct-tags integration test", () => { it("should add tags to the list", async () => { const page = shell.page(); - const tags = new Tags(page); - await tags.addTag("frontend"); - await tags.waitForTagCount(1); - await tags.addTag("javascript"); - await tags.waitForTagCount(2); - await tags.addTag("testing"); - await tags.waitForTagCount(3); - assertEquals(await tags.getTagsText(), [ - "frontend", - "javascript", - "testing", - ]); - }); - it("should not add duplicate tags", async () => { - const page = shell.page(); - const tags = new Tags(page); - assertEquals((await tags.getTags()).length, 3); + // Helper function to add a tag + const addTag = async (tagText: string) => { + // Find the add tag button + const addTagButton = await page.waitForSelector(".add-tag", { + strategy: "pierce", + }); - // Try to add a duplicate tag - await tags.addTag("frontend"); - await tags.waitForTagCount(3); - assertEquals(await tags.getTagsText(), [ - "frontend", - "javascript", - "testing", - ]); - }); + // Click using evaluate to ensure it works + await addTagButton.evaluate((el: HTMLElement) => { + el.click(); + }); + await sleep(100); // Wait for input to appear - it("should edit tags", async () => { - const page = shell.page(); - const tags = new Tags(page); - assertEquals( - (await tags.getTags()).length, - 3, - "Should still have 3 tags (no duplicates)", - ); + const addInput = await page.waitForSelector(".add-tag-input", { + strategy: "pierce", + }); + await addInput.type(tagText); + await page.keyboard.press("Enter"); + await sleep(100); // Wait for DOM update + }; - const newText = "backend"; - const tagEls = await tags.getTags(); - await tags.editTag(tagEls[0], newText); - await waitFor(() => - tags.getTags().then((els) => tags.getTagText(els[0])).then((text) => - text === newText - ) - ); - assertEquals(await tags.getTagsText(), [ - "backend", - "javascript", - "testing", - ]); - }); + // Add first tag + await addTag("frontend"); - it("should remove tags", async () => { - const page = shell.page(); - const tags = new Tags(page); - assertEquals((await tags.getTags()).length, 3); - const elements = await tags.getTags(); - await tags.removeTag(elements[0]); - await tags.waitForTagCount(2); - assertEquals(await tags.getTagsText(), [ - "javascript", - "testing", - ]); - }); + // Add second tag + await addTag("javascript"); - it("should cancel tag editing with Escape key", async () => { - const page = shell.page(); - const tags = new Tags(page); - assertEquals((await tags.getTags()).length, 2); - const originalTexts = await tags.getTagsText(); + // Add third tag + await addTag("testing"); - const elements = await tags.getTags(); - const element = elements[0]; - await element.click(); - await page.waitForSelector(".tag.editing", { strategy: "pierce" }); - const editInput = await page.waitForSelector(".tag-input", { - strategy: "pierce", + // Verify tags were added + const tags = await page.$$(".tag", { strategy: "pierce" }); + assertEquals(tags.length, 3, "Should have 3 tags"); + + // Debug: Log the structure of tags + console.log("Tag structure:"); + for (let i = 0; i < tags.length; i++) { + const tagInfo = await tags[i].evaluate( + (el: HTMLElement, idx: number) => { + const tagText = el.querySelector(".tag-text"); + const removeButton = el.querySelector(".tag-remove"); + return { + index: idx, + className: el.className, + tagText: tagText?.textContent || "no text", + hasRemoveButton: !!removeButton, + }; + }, + { args: [i] } as any, + ); + console.log(`Tag ${i}:`, tagInfo); + } + + // Verify tag content + const firstTagText = await tags[0].evaluate((el: HTMLElement) => { + const tagText = el.querySelector(".tag-text"); + return tagText?.textContent || el.textContent; }); - await editInput.evaluate((el: HTMLInputElement) => el.select()); - await editInput.type("should-be-cancelled"); - await page.keyboard.press("Escape"); - await sleep(100); - assertEquals(await tags.getTagsText(), originalTexts); + assertEquals(firstTagText?.trim(), "frontend"); + + const secondTagText = await tags[1].evaluate((el: HTMLElement) => { + const tagText = el.querySelector(".tag-text"); + return tagText?.textContent || el.textContent; + }); + assertEquals(secondTagText?.trim(), "javascript"); + + const thirdTagText = await tags[2].evaluate((el: HTMLElement) => { + const tagText = el.querySelector(".tag-text"); + return tagText?.textContent || el.textContent; + }); + assertEquals(thirdTagText?.trim(), "testing"); }); - it("should delete empty tags when backspacing", async () => { + it("should not add duplicate tags", async () => { const page = shell.page(); - const tags = new Tags(page); - assertEquals((await tags.getTags()).length, 2); - const elements = await tags.getTags(); - const element = elements[0]; - await element.click(); - await page.waitForSelector(".tag.editing", { strategy: "pierce" }); - const editInput = await page.waitForSelector(".tag-input", { - strategy: "pierce", - }); - await editInput.evaluate((el: HTMLInputElement) => el.select()); - await page.keyboard.press("Delete"); - await sleep(50); // Wait for input to be cleared - // Press Backspace on empty input - await page.keyboard.press("Backspace"); - await sleep(200); + // Helper function to add a tag + const addTag = async (tagText: string) => { + const addTagButton = await page.waitForSelector(".add-tag", { + strategy: "pierce", + }); + + await addTagButton.evaluate((el: HTMLElement) => { + el.click(); + }); + await sleep(100); + + const addInput = await page.waitForSelector(".add-tag-input", { + strategy: "pierce", + }); + await addInput.type(tagText); + await page.keyboard.press("Enter"); + await sleep(100); + }; - await tags.waitForTagCount(1); + // Try to add a duplicate tag + await addTag("frontend"); // This already exists + + // Should still have 3 tags, not 4 + const tags = await page.$$(".tag", { strategy: "pierce" }); + assertEquals(tags.length, 3, "Should still have 3 tags (no duplicates)"); }); -}); -class Tags { - #page: Page; - constructor(page: Page) { - this.#page = page; - } + it("should edit tags", async () => { + const page = shell.page(); + + console.log("Waiting for component to stabilize..."); + await sleep(500); + + // Get initial tags + const initialTags = await page.$$(".tag", { strategy: "pierce" }); + const initialCount = initialTags.length; + console.log(`Initial tag count: ${initialCount}`); + assert(initialCount > 0, "Should have tags to edit"); - getTags(): Promise { - return this.#page.$$(".tag", { strategy: "pierce" }); - } + // Click on the first tag to edit it + await initialTags[0].click(); + console.log("Clicked on first tag"); - async getTagsText(): Promise> { - const elements = await this.getTags(); - return Promise.all(elements.map((el) => this.getTagText(el))); - } + // Wait for edit mode to activate + await page.waitForSelector(".tag.editing", { strategy: "pierce" }); + console.log("Edit mode activated - found .tag.editing"); - async editTag(element: ElementHandle, newText: string) { - await element.click(); - await this.#page.waitForSelector(".tag.editing", { strategy: "pierce" }); // Look for the tag input field that appears during editing - const editInput = await this.#page.waitForSelector(".tag-input", { + const editInput = await page.waitForSelector(".tag-input", { strategy: "pierce", }); + assert(editInput, "Should find .tag-input field during editing"); // Clear and type new text await editInput.evaluate((el: HTMLInputElement) => { el.select(); // Select all text }); + const newText = "backend"; await editInput.type(newText); - await this.#page.keyboard.press("Enter"); - } - - async waitForTagCount(expected: number): Promise { - await waitFor(() => this.getTags().then((els) => els.length === expected)); - } - - async waitForTagText(element: ElementHandle, text: string): Promise { - await waitFor(() => - element.evaluate((el: HTMLInputElement) => el.value).then((value) => - value === text - ) + console.log(`Typed new text: "${newText}"`); + + // Press Enter to confirm the edit + await page.keyboard.press("Enter"); + console.log("Pressed Enter to confirm edit"); + + // Wait for the edit to be processed + await sleep(200); + + // Verify the tag was edited + const updatedTags = await page.$$(".tag", { strategy: "pierce" }); + assertEquals( + updatedTags.length, + initialCount, + "Should have same number of tags after edit", ); - } - async getTagText(element: ElementHandle): Promise { - return await element.evaluate((el: HTMLElement) => { + // Check that the first tag's text has been updated + const updatedText = await updatedTags[0].evaluate((el: HTMLElement) => { const tagText = el.querySelector(".tag-text"); return tagText?.textContent || el.textContent; }); - } + console.log(`Updated text of first tag: "${updatedText?.trim()}"`); - async addTag(text: string) { - const addTagButton = await this.#page.waitForSelector(".add-tag", { - strategy: "pierce", - }); - await addTagButton.click(); - const addInput = await this.#page.waitForSelector(".add-tag-input", { - strategy: "pierce", - }); - await addInput.type(text); - await this.#page.keyboard.press("Enter"); - } + assertEquals( + updatedText?.trim(), + newText, + "First tag should have updated text", + ); + }); + + it("should remove tags", async () => { + const page = shell.page(); + + console.log("Waiting for component to stabilize..."); + await sleep(500); + + // Get initial count + const initialTags = await page.$$(".tag", { strategy: "pierce" }); + const initialCount = initialTags.length; + console.log(`Initial tag count: ${initialCount}`); + assert(initialCount > 0, "Should have tags to remove"); - async removeTag(element: ElementHandle): Promise { // Hover over the first tag to make the remove button visible - await element.evaluate((el: HTMLElement) => { + await initialTags[0].evaluate((el: HTMLElement) => { el.dispatchEvent(new MouseEvent("mouseover", { bubbles: true })); }); + await sleep(100); // Wait for hover effect // Find and click the first remove button - const removeButtons = await this.#page.waitForSelector(".tag-remove", { + const removeButtons = await page.$$(".tag-remove", { strategy: "pierce", }); - await removeButtons.evaluate((button: HTMLElement) => { + console.log(`Found ${removeButtons.length} remove buttons`); + assert(removeButtons.length > 0, "Should find remove buttons"); + + // Debug: check what we're about to click + const buttonInfo = await removeButtons[0].evaluate((el: HTMLElement) => { + return { + className: el.className, + title: el.title, + innerText: el.innerText, + parentText: el.parentElement?.innerText || "no parent", + }; + }); + console.log("About to click remove button:", buttonInfo); + + // Click the remove button + await removeButtons[0].evaluate((button: HTMLElement) => { + console.log("About to dispatch click event on remove button:", button); button.dispatchEvent( new MouseEvent("click", { bubbles: true, @@ -247,5 +270,175 @@ class Tags { }), ); }); - } -} + console.log("Dispatched click event on first remove button"); + + // Wait for DOM to update after removal + await sleep(200); + + // Verify tag was removed + const remainingTags = await page.$$(".tag", { strategy: "pierce" }); + console.log(`After removal, found ${remainingTags.length} tags`); + + assertEquals( + remainingTags.length, + initialCount - 1, + "Should have one less tag after removal", + ); + + // Verify the first tag is now what was the second tag + if (remainingTags.length > 0) { + const firstRemainingText = await remainingTags[0].evaluate( + (el: HTMLElement) => { + const tagText = el.querySelector(".tag-text"); + return tagText?.textContent || el.textContent; + }, + ); + assertEquals( + firstRemainingText?.trim(), + "javascript", + "First tag should now be the second tag", + ); + } + }); + + it("should cancel tag editing with Escape key", async () => { + const page = shell.page(); + + console.log("Waiting for component to stabilize..."); + await sleep(500); + + // Helper function to add a tag if needed + const addTag = async (tagText: string) => { + const addTagButton = await page.waitForSelector(".add-tag", { + strategy: "pierce", + }); + + await addTagButton.evaluate((el: HTMLElement) => { + el.click(); + }); + await sleep(100); + + const addInput = await page.waitForSelector(".add-tag-input", { + strategy: "pierce", + }); + await addInput.type(tagText); + await page.keyboard.press("Enter"); + await sleep(100); + }; + + let tags = await page.$$(".tag", { strategy: "pierce" }); + + // Add a tag if none exist + if (tags.length === 0) { + await addTag("test-tag"); + tags = await page.$$(".tag", { strategy: "pierce" }); + } + + assert(tags.length > 0, "Should have tags to test escape behavior"); + + // Get the original text of the first tag + const originalText = await tags[0].evaluate((el: HTMLElement) => { + const tagText = el.querySelector(".tag-text"); + return tagText?.textContent || el.textContent; + }); + + // Click on the first tag to edit it + await tags[0].click(); + + // Wait for edit mode + await page.waitForSelector(".tag.editing", { strategy: "pierce" }); + + // Find the edit input and type some text + const editInput = await page.waitForSelector(".tag-input", { + strategy: "pierce", + }); + await editInput.evaluate((el: HTMLInputElement) => { + el.select(); + }); + await editInput.type("should-be-cancelled"); + + // Press Escape to cancel + await page.keyboard.press("Escape"); + await sleep(100); + + // Verify the edit was cancelled and original text is preserved + const updatedTags = await page.$$(".tag", { strategy: "pierce" }); + const currentText = await updatedTags[0].evaluate((el: HTMLElement) => { + const tagText = el.querySelector(".tag-text"); + return tagText?.textContent || el.textContent; + }); + + assertEquals( + currentText?.trim(), + originalText?.trim(), + "Tag text should be unchanged after Escape", + ); + }); + + it("should delete empty tags when backspacing", async () => { + const page = shell.page(); + + console.log("Waiting for component to stabilize..."); + await sleep(500); + + // Helper function to add a tag if needed + const addTag = async (tagText: string) => { + const addTagButton = await page.waitForSelector(".add-tag", { + strategy: "pierce", + }); + + await addTagButton.evaluate((el: HTMLElement) => { + el.click(); + }); + await sleep(100); + + const addInput = await page.waitForSelector(".add-tag-input", { + strategy: "pierce", + }); + await addInput.type(tagText); + await page.keyboard.press("Enter"); + await sleep(100); + }; + + // Get initial count + let initialTags = await page.$$(".tag", { strategy: "pierce" }); + + // Add a tag if none exist + if (initialTags.length === 0) { + await addTag("test-tag"); + initialTags = await page.$$(".tag", { strategy: "pierce" }); + } + + const initialCount = initialTags.length; + + // Click on the first tag to edit it + await initialTags[0].click(); + + // Wait for edit mode + await page.waitForSelector(".tag.editing", { strategy: "pierce" }); + + // Find the edit input and clear all text using keyboard + const editInput = await page.waitForSelector(".tag-input", { + strategy: "pierce", + }); + + // Select all text and delete it + await editInput.evaluate((el: HTMLInputElement) => { + el.select(); + }); + await page.keyboard.press("Delete"); + await sleep(50); // Wait for input to be cleared + + // Press Backspace on empty input + await page.keyboard.press("Backspace"); + await sleep(200); + + // Verify the tag was removed + const remainingTags = await page.$$(".tag", { strategy: "pierce" }); + assertEquals( + remainingTags.length, + initialCount - 1, + "Should have one less tag after backspacing empty tag", + ); + }); +}); diff --git a/packages/patterns/integration/instantiate-recipe.test.ts b/packages/patterns/integration/instantiate-recipe.test.ts index 754a351779..1f148596f0 100644 --- a/packages/patterns/integration/instantiate-recipe.test.ts +++ b/packages/patterns/integration/instantiate-recipe.test.ts @@ -9,9 +9,6 @@ import { CharmsController } from "@commontools/charm/ops"; const { API_URL, FRONTEND_URL, SPACE_NAME } = env; -// TODO(CT-1101) Need to re-enable these tests to make them more robust -const ignore = true; - describe("instantiate-recipe integration test", () => { const shell = new ShellIntegration(); shell.bindLifecycle(); @@ -44,65 +41,61 @@ describe("instantiate-recipe integration test", () => { if (cc) await cc.dispose(); }); - it({ - name: "should deploy recipe, click button, and navigate to counter", - ignore, - fn: async () => { - const page = shell.page(); - - await shell.goto({ - frontendUrl: FRONTEND_URL, - view: { - spaceName: SPACE_NAME, - charmId, - }, - identity, - }); - - // Wait for charm to load by waiting for first interactive element - await page.waitForSelector("[data-ct-input]", { strategy: "pierce" }); - - // Store the current URL before any action - const urlBefore = await page.evaluate(() => globalThis.location.href); - console.log("URL before action:", urlBefore); - - const input = await page.waitForSelector("[data-ct-input]", { - strategy: "pierce", - }); - - await input.type("New counter"); - - // Quick wait for input processing - await sleep(100); - - const button = await page.waitForSelector("[data-ct-button]", { - strategy: "pierce", - }); - - await button.click(); - - // Wait for page to soft navigate - await page.waitForFunction((urlBefore) => { - return globalThis.location.href !== urlBefore; - }, { args: [urlBefore] }); - - const urlAfter = await page.evaluate(() => globalThis.location.href); - console.log("URL after clicking:", urlAfter); - - // Verify navigation happened (URL should have changed) - assert( - urlBefore !== urlAfter, - "Should navigate to a new URL after clicking Add button", - ); - - // Verify we're now on a counter page by checking for counter-specific elements - const counterResult = await page.waitForSelector("#counter-result", { - strategy: "pierce", - }); - assert( - counterResult, - "Should find counter-result element after navigation", - ); - }, + it("should deploy recipe, click button, and navigate to counter", async () => { + const page = shell.page(); + + await shell.goto({ + frontendUrl: FRONTEND_URL, + view: { + spaceName: SPACE_NAME, + charmId, + }, + identity, + }); + + // Wait for charm to load by waiting for first interactive element + await page.waitForSelector("[data-ct-input]", { strategy: "pierce" }); + + // Store the current URL before any action + const urlBefore = await page.evaluate(() => globalThis.location.href); + console.log("URL before action:", urlBefore); + + const input = await page.waitForSelector("[data-ct-input]", { + strategy: "pierce", + }); + + await input.type("New counter"); + + // Quick wait for input processing + await sleep(100); + + const button = await page.waitForSelector("[data-ct-button]", { + strategy: "pierce", + }); + + await button.click(); + + // Wait for page to soft navigate + await page.waitForFunction((urlBefore) => { + return globalThis.location.href !== urlBefore; + }, { args: [urlBefore] }); + + const urlAfter = await page.evaluate(() => globalThis.location.href); + console.log("URL after clicking:", urlAfter); + + // Verify navigation happened (URL should have changed) + assert( + urlBefore !== urlAfter, + "Should navigate to a new URL after clicking Add button", + ); + + // Verify we're now on a counter page by checking for counter-specific elements + const counterResult = await page.waitForSelector("#counter-result", { + strategy: "pierce", + }); + assert( + counterResult, + "Should find counter-result element after navigation", + ); }); }); diff --git a/packages/patterns/integration/list-operations.test.ts b/packages/patterns/integration/list-operations.test.ts index 8e8ccb8217..9d7d0b5cf8 100644 --- a/packages/patterns/integration/list-operations.test.ts +++ b/packages/patterns/integration/list-operations.test.ts @@ -3,7 +3,7 @@ import { CharmController, CharmsController } from "@commontools/charm/ops"; import { ShellIntegration } from "@commontools/integration/shell-utils"; import { afterAll, beforeAll, describe, it } from "@std/testing/bdd"; import { join } from "@std/path"; -import { assertEquals } from "@std/assert"; +import { assert, assertEquals } from "@std/assert"; import { Identity } from "@commontools/identity"; const { API_URL, FRONTEND_URL, SPACE_NAME } = env; @@ -54,27 +54,23 @@ describe("list-operations simple test", () => { await page.waitForSelector("#main-list", { strategy: "pierce" }); // Click reset to populate with initial data - const resetBtn = await page.waitForSelector("#reset-demo", { - strategy: "pierce", - }); + const resetBtn = await page.$("#reset-demo", { strategy: "pierce" }); + assert(resetBtn, "Should find reset button"); await resetBtn.click(); // Wait for the reset operation to complete by checking the text content + const mainList = await page.$("#main-list", { strategy: "pierce" }); + assert(mainList, "Should find main list element"); + await waitFor(async () => { - const mainList = await page.waitForSelector("#main-list", { - strategy: "pierce", - }); - const initialText = await mainList.evaluate((el: HTMLElement) => + const initialText = await mainList!.evaluate((el: HTMLElement) => el.textContent || "" ); return initialText === "A, B, C, D (4)"; }); // Verify the list populated correctly - const mainList = await page.waitForSelector("#main-list", { - strategy: "pierce", - }); - const initialText = await mainList.evaluate((el: HTMLElement) => + const initialText = await mainList!.evaluate((el: HTMLElement) => el.textContent || "" ); assertEquals( @@ -84,26 +80,24 @@ describe("list-operations simple test", () => { ); // Test delete first item - const deleteFirstBtn = await page.waitForSelector("#delete-first", { + const deleteFirstBtn = await page.$("#delete-first", { strategy: "pierce", }); - await deleteFirstBtn.click(); + await deleteFirstBtn!.click(); // Wait for delete to complete await waitFor(async () => { - const currentMainList = await page.waitForSelector("#main-list", { + const currentMainList = await page.$("#main-list", { strategy: "pierce", }); - const text = await currentMainList.evaluate((el: HTMLElement) => + const text = await currentMainList!.evaluate((el: HTMLElement) => el.textContent || "" ); return text === "B, C, D (3)"; }); - const currentMainList = await page.waitForSelector("#main-list", { - strategy: "pierce", - }); - const afterDeleteText = await currentMainList.evaluate((el: HTMLElement) => + const currentMainList = await page.$("#main-list", { strategy: "pierce" }); + const afterDeleteText = await currentMainList!.evaluate((el: HTMLElement) => el.textContent || "" ); assertEquals( @@ -113,26 +107,22 @@ describe("list-operations simple test", () => { ); // Test insert at start - const insertStartBtn = await page.waitForSelector("#insert-start", { + const insertStartBtn = await page.$("#insert-start", { strategy: "pierce", }); - await insertStartBtn.click(); + await insertStartBtn!.click(); // Wait for insert to complete await waitFor(async () => { - const insertMainList = await page.waitForSelector("#main-list", { - strategy: "pierce", - }); - const text = await insertMainList.evaluate((el: HTMLElement) => + const insertMainList = await page.$("#main-list", { strategy: "pierce" }); + const text = await insertMainList!.evaluate((el: HTMLElement) => el.textContent || "" ); return text === "New Start, B, C, D (4)"; }); - const insertMainList = await page.waitForSelector("#main-list", { - strategy: "pierce", - }); - const afterInsertText = await insertMainList.evaluate((el: HTMLElement) => + const insertMainList = await page.$("#main-list", { strategy: "pierce" }); + const afterInsertText = await insertMainList!.evaluate((el: HTMLElement) => el.textContent || "" ); assertEquals( @@ -142,25 +132,21 @@ describe("list-operations simple test", () => { ); // Test one more operation: delete-last to see if it works - const deleteLastBtn = await page.waitForSelector("#delete-last", { - strategy: "pierce", - }); - await deleteLastBtn.click(); + const deleteLastBtn = await page.$("#delete-last", { strategy: "pierce" }); + await deleteLastBtn!.click(); await waitFor(async () => { - const deleteLastMainList = await page.waitForSelector("#main-list", { + const deleteLastMainList = await page.$("#main-list", { strategy: "pierce", }); - const text = await deleteLastMainList.evaluate((el: HTMLElement) => + const text = await deleteLastMainList!.evaluate((el: HTMLElement) => el.textContent || "" ); return text === "New Start, B, C (3)"; }); - const finalMainList = await page.waitForSelector("#main-list", { - strategy: "pierce", - }); - const finalText = await finalMainList.evaluate((el: HTMLElement) => + const finalMainList = await page.$("#main-list", { strategy: "pierce" }); + const finalText = await finalMainList!.evaluate((el: HTMLElement) => el.textContent || "" ); assertEquals( diff --git a/packages/patterns/integration/nested-counter.test.ts b/packages/patterns/integration/nested-counter.test.ts index 596846a029..b32a519c08 100644 --- a/packages/patterns/integration/nested-counter.test.ts +++ b/packages/patterns/integration/nested-counter.test.ts @@ -51,15 +51,16 @@ describe("nested counter integration test", () => { identity, }); - await waitFor(async () => { - const counterResult = await page.waitForSelector("#counter-result", { - strategy: "pierce", - }); - const initialText = await counterResult.evaluate((el: HTMLElement) => - el.textContent - ); - return initialText?.trim() === "Counter is the 0th number"; + const counterResult = await page.waitForSelector("#counter-result", { + strategy: "pierce", }); + assert(counterResult, "Should find counter-result element"); + + // Verify initial value is 0 + const initialText = await counterResult.evaluate((el: HTMLElement) => + el.textContent + ); + assertEquals(initialText?.trim(), "Counter is the 0th number"); // Verify via direct operations that the nested structure works assertEquals(charm.result.get(["value"]), 0); diff --git a/packages/shell/integration/charm.test.ts b/packages/shell/integration/charm.test.ts index b9aaea34a9..3401b7038a 100644 --- a/packages/shell/integration/charm.test.ts +++ b/packages/shell/integration/charm.test.ts @@ -59,25 +59,20 @@ describe("shell charm tests", () => { identity, }); - // Click twice - for (let i = 0; i < 2; i++) { - const handle = await page.waitForSelector( - "#counter-decrement", - { strategy: "pierce" }, - ); - // Wait for inner text. There's some - // race here where we can click before the - // box model is available. - const _ = await handle.innerText(); - handle.click(); - await sleep(1000); - } - const handle = await page.waitForSelector( + let handle = await page.waitForSelector( + "#counter-decrement", + { strategy: "pierce" }, + ); + handle.click(); + await sleep(1000); + handle.click(); + await sleep(1000); + handle = await page.waitForSelector( "#counter-result", { strategy: "pierce" }, ); - await sleep(1500); - const text = await handle.innerText(); + await sleep(1000); + const text = await handle?.innerText(); assert(text === "Counter is the -2th number"); }); }); diff --git a/packages/shell/integration/iframe-counter-charm.disabled_test.ts b/packages/shell/integration/iframe-counter-charm.disabled_test.ts index 3cead41638..e209748aba 100644 --- a/packages/shell/integration/iframe-counter-charm.disabled_test.ts +++ b/packages/shell/integration/iframe-counter-charm.disabled_test.ts @@ -65,7 +65,7 @@ async function getCharmResult(page: Page): Promise<{ count: number }> { // Use the element handle to evaluate in its context return await appView.evaluate((element: XAppView) => { // Access the private _activeCharm property - const activeCharmTask = element._patterns; + const activeCharmTask = element._activePatterns; if (!activeCharmTask) { throw new Error("No _activeCharm property found on element"); @@ -77,9 +77,6 @@ async function getCharmResult(page: Page): Promise<{ count: number }> { // Get the charm controller from the Task's value const charmController = activeCharmTask.value.activePattern; - if (!charmController) { - throw new Error("No active charm controller found"); - } // Get the result from the charm controller const result = charmController.result.get(); diff --git a/packages/shell/src/lib/pattern-factory.ts b/packages/shell/src/lib/pattern-factory.ts index 28bdc17a46..7e92045f70 100644 --- a/packages/shell/src/lib/pattern-factory.ts +++ b/packages/shell/src/lib/pattern-factory.ts @@ -1,9 +1,8 @@ import { CharmController, CharmsController } from "@commontools/charm/ops"; import { HttpProgramResolver } from "@commontools/js-compiler"; import { API_URL } from "./env.ts"; -import { NameSchema } from "@commontools/charm"; -export type BuiltinPatternType = "home" | "space-root"; +export type BuiltinPatternType = "home" | "space-default"; type BuiltinPatternConfig = { url: URL; @@ -17,17 +16,17 @@ const Configs: Record = { url: new URL(`/api/patterns/home.tsx`, API_URL), cause: "home-pattern", }, - "space-root": { + "space-default": { name: "DefaultCharmList", url: new URL(`/api/patterns/default-app.tsx`, API_URL), - cause: "space-root", + cause: "default-charm", }, }; export async function create( cc: CharmsController, type: BuiltinPatternType, -): Promise> { +): Promise { const config = Configs[type]; const manager = cc.manager(); const runtime = manager.runtime; @@ -36,11 +35,7 @@ export async function create( new HttpProgramResolver(config.url.href), ); - const charm = await cc.create( - program, - { start: true }, - config.cause, - ); + const charm = await cc.create(program, { start: true }, config.cause); // Wait for the link to be processed await runtime.idle(); @@ -54,7 +49,7 @@ export async function create( export async function get( cc: CharmsController, -): Promise | undefined> { +): Promise { const pattern = await cc.manager().getDefaultPattern(); if (!pattern) { return undefined; @@ -65,7 +60,7 @@ export async function get( export async function getOrCreate( cc: CharmsController, type: BuiltinPatternType, -): Promise> { +): Promise { const pattern = await get(cc); if (pattern) { return pattern; diff --git a/packages/shell/src/lib/runtime.ts b/packages/shell/src/lib/runtime.ts index e2e135813a..0a9b8a89c1 100644 --- a/packages/shell/src/lib/runtime.ts +++ b/packages/shell/src/lib/runtime.ts @@ -5,20 +5,14 @@ import { RuntimeTelemetryEvent, RuntimeTelemetryMarkerResult, } from "@commontools/runner"; -import { - charmId, - CharmManager, - NameSchema, - nameSchema, -} from "@commontools/charm"; -import { CharmController, CharmsController } from "@commontools/charm/ops"; +import { charmId, CharmManager } from "@commontools/charm"; +import { CharmsController } from "@commontools/charm/ops"; import { StorageManager } from "@commontools/runner/storage/cache"; import { navigate } from "./navigate.ts"; import * as Inspector from "@commontools/runner/storage/inspector"; import { setupIframe } from "./iframe-ctx.ts"; import { getLogger } from "@commontools/utils/logger"; import { AppView } from "./app/view.ts"; -import * as PatternFactory from "./pattern-factory.ts"; const logger = getLogger("shell.telemetry", { enabled: false, @@ -40,20 +34,15 @@ export class RuntimeInternals extends EventTarget { #inspector: Inspector.Channel; #disposed = false; #space: string; // The MemorySpace DID - #spaceRootPattern?: CharmController; - #spaceRootPatternType: PatternFactory.BuiltinPatternType; - #patternCache: Map> = new Map(); private constructor( cc: CharmsController, telemetry: RuntimeTelemetry, space: string, - spaceRootPatternType: PatternFactory.BuiltinPatternType, ) { super(); this.#cc = cc; this.#space = space; - this.#spaceRootPatternType = spaceRootPatternType; const runtimeId = this.#cc.manager().runtime.id; this.#inspector = new Inspector.Channel( runtimeId, @@ -80,45 +69,6 @@ export class RuntimeInternals extends EventTarget { return this.#space; } - // Returns the space root pattern, creating it if it doesn't exist. - // The space root pattern type is determined at RuntimeInternals creation - // based on the view type (home vs space). - async getSpaceRootPattern(): Promise> { - this.#check(); - if (this.#spaceRootPattern) { - return this.#spaceRootPattern; - } - - const pattern = await PatternFactory.getOrCreate( - this.#cc, - this.#spaceRootPatternType, - ); - - // Track as recently accessed - await this.#cc.manager().trackRecentCharm(pattern.getCell()); - - this.#patternCache.set(pattern.id, pattern); - return pattern; - } - - // Returns a pattern by ID, starting it if requested. - // Patterns are cached by ID. - async getPattern(id: string): Promise> { - this.#check(); - const cached = this.#patternCache.get(id); - if (cached) { - return cached; - } - - const pattern = await this.#cc.get(id, true, nameSchema); - - // Track as recently accessed - await this.#cc.manager().trackRecentCharm(pattern.getCell()); - - this.#patternCache.set(id, pattern); - return pattern; - } - async dispose() { if (this.#disposed) return; this.#disposed = true; @@ -156,24 +106,20 @@ export class RuntimeInternals extends EventTarget { ): Promise { let session; let spaceName; - let spaceRootPatternType: PatternFactory.BuiltinPatternType; if ("builtin" in view) { switch (view.builtin) { case "home": session = await createSession({ identity, spaceDid: identity.did() }); spaceName = ""; - spaceRootPatternType = "home"; break; } } else if ("spaceName" in view) { session = await createSession({ identity, spaceName: view.spaceName }); spaceName = view.spaceName; - spaceRootPatternType = "space-root"; } else if ("spaceDid" in view) { session = await createSession({ identity, spaceDid: view.spaceDid }); - spaceRootPatternType = "space-root"; } - if (!session || !spaceRootPatternType!) { + if (!session) { throw new Error("Unexpected view provided."); } @@ -288,11 +234,6 @@ export class RuntimeInternals extends EventTarget { charmManager = new CharmManager(session, runtime); await charmManager.synced(); const cc = new CharmsController(charmManager); - return new RuntimeInternals( - cc, - telemetry, - session.space, - spaceRootPatternType, - ); + return new RuntimeInternals(cc, telemetry, session.space); } } diff --git a/packages/shell/src/views/AppView.ts b/packages/shell/src/views/AppView.ts index 39dfd90a2b..a29d5e00fa 100644 --- a/packages/shell/src/views/AppView.ts +++ b/packages/shell/src/views/AppView.ts @@ -1,17 +1,20 @@ import { css, html } from "lit"; import { property, state } from "lit/decorators.js"; + +import { AppView } from "../lib/app/mod.ts"; import { BaseView, createDefaultAppState } from "./BaseView.ts"; import { KeyStore } from "@commontools/identity"; import { RuntimeInternals } from "../lib/runtime.ts"; import { DebuggerController } from "../lib/debugger-controller.ts"; import "./DebuggerView.ts"; -import { Task, TaskStatus } from "@lit/task"; -import { CharmController } from "@commontools/charm/ops"; +import { Task } from "@lit/task"; +import { CharmController, CharmsController } from "@commontools/charm/ops"; import { CellEventTarget, CellUpdateEvent } from "../lib/cell-event-target.ts"; import { NAME } from "@commontools/runner"; -import { type NameSchema } from "@commontools/charm"; +import { type NameSchema, nameSchema } from "@commontools/charm"; import { updatePageTitle } from "../lib/navigate.ts"; import { KeyboardController } from "../lib/keyboard-router.ts"; +import * as PatternFactory from "../lib/pattern-factory.ts"; export class XAppView extends BaseView { static override styles = css` @@ -49,7 +52,7 @@ export class XAppView extends BaseView { @property({ attribute: false }) keyStore?: KeyStore; - @state() + @property({ attribute: false }) charmTitle?: string; @property({ attribute: false }) @@ -82,78 +85,43 @@ export class XAppView extends BaseView { this.hasSidebarContent = event.detail.hasSidebarContent; }; - // Fetches the space root pattern from the space. - _spaceRootPattern = new Task(this, { - task: async ( - [rt], - ): Promise< - | CharmController - | undefined - > => { - if (!rt) return; - return await rt.getSpaceRootPattern(); - }, - args: () => [this.rt], - }); - - // This fetches the selected pattern, the explicitly chosen pattern - // to render via URL e.g. `/space/someCharmId`. - _selectedPattern = new Task(this, { + // Do not make private, integration tests access this directly. + // + // This fetches the active pattern and space default pattern derived + // from the current view. + _activePatterns = new Task(this, { task: async ( [app, rt], ): Promise< - | CharmController + | { activePattern: CharmController; defaultPattern: CharmController } | undefined > => { - if (!rt) return; - if ("charmId" in app.view && app.view.charmId) { - return await rt.getPattern(app.view.charmId); + if (!rt) { + this.#setTitleSubscription(); + return; + } + + const patterns = await viewToPatterns( + rt.cc(), + app.view, + this._activePatterns.value?.activePattern, + ); + if (!patterns) { + this.#setTitleSubscription(); + return; } - }, - args: () => [this.app, this.rt], - }); - // This derives a space root pattern as well as an "active" (main) - // pattern for use in child views. - // This hybrid task intentionally only uses completed/fresh - // source patterns to avoid unsyncing state. - _patterns = new Task(this, { - task: function ( - [ - app, - spaceRootPatternValue, - spaceRootPatternStatus, - selectedPatternValue, - selectedPatternStatus, - ], - ): { - activePattern: CharmController | undefined; - spaceRootPattern: CharmController | undefined; - } { - const spaceRootPattern = spaceRootPatternStatus === TaskStatus.COMPLETE - ? spaceRootPatternValue - : undefined; - // The "active" pattern is the main pattern to be rendered. - // This may be the same as the space root pattern, unless we're - // in a view that specifies a different pattern to use. - const useSpaceRootAsActive = !("charmId" in app.view && app.view.charmId); - const activePattern = useSpaceRootAsActive - ? spaceRootPattern - : selectedPatternStatus === TaskStatus.COMPLETE - ? selectedPatternValue - : undefined; - return { - activePattern, - spaceRootPattern, - }; + // Record the charm as recently accessed so recents stay fresh. + await rt.cc().manager().trackRecentCharm( + patterns.activePattern.getCell(), + ); + this.#setTitleSubscription( + patterns.activePattern as CharmController, + ); + + return patterns; }, - args: () => [ - this.app, - this._spaceRootPattern.value, - this._spaceRootPattern.status, - this._selectedPattern.value, - this._selectedPattern.status, - ], + args: () => [this.app, this.rt], }); #setTitleSubscription(activeCharm?: CharmController) { @@ -206,42 +174,31 @@ export class XAppView extends BaseView { } // Update debugger visibility from app state - if (changedProperties.has("app")) { + if (changedProperties.has("app") && this.app) { this.debuggerController.setVisibility( this.app.config.showDebuggerView ?? false, ); } } - // Always defer to the loaded active pattern for the ID, - // but until that loads, use an ID in the view if available. - private getActivePatternId(): string | undefined { - const activePattern = this._patterns.value?.activePattern; - if (activePattern?.id) return activePattern.id; - if ("charmId" in this.app.view && this.app.view.charmId) { - return this.app.view.charmId; - } - } - override render() { const config = this.app.config ?? {}; - const { activePattern, spaceRootPattern } = this._patterns.value ?? {}; - this.#setTitleSubscription(activePattern); - + const unauthenticated = html` + + `; + const patterns = this._activePatterns.value; + const activePattern = patterns?.activePattern; + const defaultPattern = patterns?.defaultPattern; const authenticated = html` `; - const unauthenticated = html` - - `; - const charmId = this.getActivePatternId(); const spaceName = this.app && "spaceName" in this.app.view ? this.app.view.spaceName : undefined; @@ -254,7 +211,7 @@ export class XAppView extends BaseView { .rt="${this.rt}" .keyStore="${this.keyStore}" .charmTitle="${this.charmTitle}" - .charmId="${charmId}" + .charmId="${activePattern?.id}" .showShellCharmListView="${config.showShellCharmListView ?? false}" .showDebuggerView="${config.showDebuggerView ?? false}" .showSidebar="${config.showSidebar ?? false}" @@ -264,7 +221,7 @@ export class XAppView extends BaseView { ${content} - ${this.app.identity + ${this.app?.identity ? html` , +): Promise< + { + activePattern: CharmController; + defaultPattern: CharmController; + } | undefined +> { + if ("builtin" in view) { + if (view.builtin !== "home") { + console.warn("Unsupported view type"); + return; + } + const pattern = await PatternFactory.getOrCreate(cc, "home"); + return { activePattern: pattern, defaultPattern: pattern }; + } else if ("spaceDid" in view) { + console.warn("Unsupported view type"); + return; + } else if ("spaceName" in view) { + const defaultPattern = await PatternFactory.getOrCreate( + cc, + "space-default", + ); + + let activePattern: CharmController | undefined; + // If viewing a specific charm, use it as active; otherwise use default + if (view.charmId) { + if (currentActive && currentActive.id === view.charmId) { + activePattern = currentActive; + } else { + activePattern = await cc.get( + view.charmId, + true, + nameSchema, + ); + } + } else { + activePattern = defaultPattern; + } + return { activePattern, defaultPattern }; + } +} + globalThis.customElements.define("x-app-view", XAppView); diff --git a/packages/shell/src/views/BodyView.ts b/packages/shell/src/views/BodyView.ts index f3f596a09a..8b961247f7 100644 --- a/packages/shell/src/views/BodyView.ts +++ b/packages/shell/src/views/BodyView.ts @@ -45,10 +45,10 @@ export class XBodyView extends BaseView { rt?: RuntimeInternals; @property({ attribute: false }) - activePattern?: CharmController; + activeCharm?: CharmController; @property({ attribute: false }) - spaceRootPattern?: CharmController; + defaultCharm?: CharmController; @property() showShellCharmListView = false; @@ -96,16 +96,16 @@ export class XBodyView extends BaseView { `; } - const mainContent = this.activePattern + const mainContent = this.activeCharm ? html` - - + + ` : null; - const sidebarCell = this.activePattern?.getCell().key("sidebarUI"); - const fabCell = this.spaceRootPattern?.getCell().key("fabUI"); + const sidebarCell = this.activeCharm?.getCell().key("sidebarUI"); + const fabCell = this.defaultCharm?.getCell().key("fabUI"); // Update sidebar content detection // TODO(seefeld): Fix possible race here where charm is already set, but diff --git a/packages/shell/src/views/RootView.ts b/packages/shell/src/views/RootView.ts index 19cdebdd80..363198ddb7 100644 --- a/packages/shell/src/views/RootView.ts +++ b/packages/shell/src/views/RootView.ts @@ -17,10 +17,9 @@ import { runtimeContext, spaceContext } from "@commontools/ui"; import { provide } from "@lit/context"; // The root element for the shell application. -// -// Derives `RuntimeInternals` for the application from its `AppState`. -// `Command` mutates the app state, which can be fired as events -// from children elements. +// Handles processing `Command`s from children elements, +// updating the `AppState`, and providing changes +// to children elements. export class XRootView extends BaseView { static override styles = css` :host {