Log Viewer: Modernize with new @observedFrom decorator#22520
Log Viewer: Modernize with new @observedFrom decorator#22520iOvergaard wants to merge 13 commits intomainfrom
Conversation
Replaces queueMicrotask in @consumeContext and @Providecontext with a hostConnected controller. The controller is only created when the element connects to the DOM, which guarantees: 1. All class field initializers have run (no undefined private fields) 2. The element is in the DOM, so context-request events can dispatch 3. Resolution happens before the first render if a provider is already in the ancestor tree (no flash of undefined for common case) Previously queueMicrotask could fire before or after first render depending on the event loop, making timing unreliable. Using the controller host protocol ties resolution to a well-defined lifecycle phase. All existing tests (6 consume + 4 provide) pass unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- consume: asserts context resolves before first render (undefined never appears in rendered values) - consume: asserts late-arriving provider is picked up correctly - consume: asserts disconnect/reconnect doesn't duplicate the consumer - provide: asserts descendants see the context at first render These guard against regressions in the hostConnected-based timing introduced in the previous commit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Aligns UmbControllerHostMixin with Lit's ReactiveElement pattern by lazy-initializing the controllers list (and the `_attached` flag) using TypeScript `private` + `??=` — the same approach Lit uses for its own `__controllers` set. With that in place, `addUmbController` is safe from Lit's `addInitializer` callbacks (which fire before inherited class-field initializers), removing the need for the throwaway Lit ReactiveController wrapper previously used by `@consumeContext` and `@provideContext`. Addresses the PR review feedback: - Niels: "Lit Controllers to add an Umb Controller 😵💫" — gone. The decorators now attach UmbContextConsumer/ProviderController directly. - Copilot #1/#3: `this.addController(...)` throws on non-Lit hosts (e.g. UmbControllerBase). Unified on `addUmbController`, which exists on every UmbControllerHost. - Copilot #2: `asPromise().then(...)` could reject with no `.catch`, surfacing as unhandled rejection when `subscribe: false` and no provider resolves. Now swallowed; added an explicit test. - Copilot #4: the "provide to descendants on first render" test only asserted after `elementUpdated`. Tightened to capture values inside `render()` and assert the first rendered value. - Copilot #5: the "no duplicate consumer on disconnect/reconnect" test didn't actually count controllers. Now counts `UmbContextConsumerController` instances on the host before and after reconnect. Also added a dedicated test in `controller.test.ts` for calling `addUmbController` from `addInitializer` to lock the new timing guarantee in.
New primitives that collapse the "consume context → observe slice → render"
pattern into one declaration.
- @observedFrom(token, selector, { default }) accessor/property decorator
that binds the decorated property to an observable slice of a context.
Supports both standard (TC39 Stage 3) and legacy (TS experimental)
decorator forms. Uses a stable controller alias so re-provisioning
doesn't leak observer controllers.
- this.observeContext(token, selector, callback) method on UmbClassMixin
and UmbElementMixin. Combines consumeContext + observe for side-effect
observations that don't bind to a property.
- Also simplifies existing @consumeContext / @Providecontext decorators:
replaces the "add a Lit controller that creates an UmbController" pattern
with direct queueMicrotask, addressing inline review feedback
(mixing Lit and Umb controllers is confusing; microtask is simpler).
Adds 8 unit tests for @observedFrom and this.observeContext covering:
- binding to observable slice, re-rendering on emit
- default value before resolution
- late-arriving provider
- re-subscription on new closer provider
- unprovide cleanup
All 73 existing context-api tests pass unchanged.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Applies the new primitives to the five simplest context-consuming files: - log-viewer-polling-button.element.ts - log-viewer-date-range-selector.element.ts (uses this.observeContext) - log-viewer-log-level-overview.element.ts (uses callback option on @consumeContext) - log-viewer-log-level-filter-menu.element.ts - log-overview-view.element.ts (uses callback option on @consumeContext) Net -61 lines across 5 files. Each file loses the setter-based @consumeContext pattern, its manual #observe* method, and its backing field — replaced by a single @observedFrom declaration per observed slice. Side-effect context method calls (getLogLevels, getSavedSearches) moved to the @consumeContext `callback` option instead of living in a setter. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- log-viewer-saved-searches-overview.element.ts - log-viewer-message-templates-overview.element.ts The paged views bind the full paged response via @observedFrom, then derive `_items` and `_total` through simple getters. Initial fetch is triggered via this.consumeContext in the constructor (needs `this` for pagination state, which a plain callback can't access). Net -18 lines across 2 files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- log-viewer-messages-list.element.ts — 4 @observedFrom bindings for sortingDirection/logs/logsTotal/isLoading; this.observeContext for the filterExpression side-effect (skip(1) → getLogs) - log-viewer-search-input.element.ts — @observedFrom for savedSearches and filterExpression. _isQuerySaved becomes a derived getter computed from the observed state, removing an assignment that previously had to be manually synced on save. Side effects (getSavedSearches on init) moved to @consumeContext callback. Net -40 lines across 2 files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The motivating example — 107 lines → 84 lines. Replaces 7 pieces of machinery (backing field, setter, getter, observer field, observe method, manual .destroy() cleanup, @State field) with a single @observedFrom declaration. Removes the pre-controller-aliases manual cleanup idiom. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The final context-consuming file. @observedFrom handles dateRange and logCount; this.consumeContext in constructor handles the initial getLogCount() side-effect. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…From When an observable emits undefined (common with asObservablePart selectors over nullable state), the property should fall back to the decorator's default value rather than becoming undefined itself. This matches the defensive \`value ?? default\` pattern consumers wrote manually and prevents TypeError: Cannot read properties of undefined in render methods. Verified end-to-end in the Log Viewer: overview loads with log counts, common messages, saved searches. Search view renders messages with pagination. Filter input, polling toggle, date range selector all work. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove unused _logViewerContext field in log-types-chart (only used briefly in the consumeContext callback, now inlined) - Drop redundant "what-the-code-does" comments - Clean up observerAlias resolution in observeContext helpers: collapse the double ternary + ?? null chain into one clearer expression Skipped (not worth fixing here): - Decorator structure duplication vs consumeContext (would require extracting a shared helper; separate refactor) - _savedSearchesPage/_messageTemplatesPage getter pattern repeated across 3 files (would want a UmbPageResult helper type; scope creep) - Mixin duplication between class.mixin and element.mixin (pre-existing pattern for all context/observe methods; consistent is consistent) - Multiple @observedFrom per context creates N consumers (acceptable overhead; documented in PR description) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tConnected strategy Addresses review findings: - New test verifies the `default` option is re-applied when the observable emits `undefined` (common with `asObservablePart` selectors over nullable state; mirrors defensive `value ?? default` pattern). - New test exercises the UmbControllerBase hostConnected-wrapper path (strategy 2) which was not previously covered. Also clarifies the JSDoc timing description: setup is deferred one microtask so inherited private fields are initialized before the consumer registers; first render may therefore see the default before the context resolves.
With UmbControllerHostMixin now lazy-initializing its controllers list (per the parent-branch refactor), @observedFrom no longer needs queueMicrotask deferral in either the standard or legacy decorator path — the UmbContextConsumerController can register immediately via addUmbController. Also extracts the shared consumer-bind logic into a `bindObservedFrom` helper so the standard, legacy-addInitializer, and legacy-hostConnected-wrap paths all go through one place. Swaps the private `__observedFromSetup: Set<string>` bag on instances for a per-site `Symbol` marker, matching the pattern used by @consumeContext and @Providecontext legacy strategy 2.
a8515d4 to
26876bf
Compare
| const initialValue = element[propertyKey]; | ||
| new UmbContextProviderController<BaseType, ResultType, InstanceType>(element, context, initialValue); | ||
| let initialized = false; | ||
| element.addUmbController({ |
There was a problem hiding this comment.
So this adds a controller, to add the controller?
Seems like there is a problem, that this tries to avoid that that should be resolved in other ways?
| #controllers: UmbController[] = []; | ||
|
|
||
| #attached = false; | ||
| // Declared as TS `private` without an initializer so access is safe |
There was a problem hiding this comment.
Are you saying this fix some JS flow, so controllers can be added as part of the constructor?
| subscribe: boolean, | ||
| ): void { | ||
| if (subscribe) { | ||
| new UmbContextConsumerController(host, context, (value) => { |
There was a problem hiding this comment.
I´ve had this particular idea as well, but what held me back from doing it so far have been this problem.
Imagine this case:
`
@observedFrom(UMB_APP_LOG_VIEWER_CONTEXT, (ctx) => ctx.logs, { default: [] })
@State() private _logs: LogMessageResponseModel[] = [];
@observedFrom(UMB_APP_LOG_VIEWER_CONTEXT, (ctx) => ctx.something, { default: [] })
@State() private _something: LogMessageResponseModel[] = [];
`
This would create two Context Consumers, which is a performance problem, Context Consumptions are the heaviest JS task of the application and we need in our code and for Packages code to keep it as low as posible.
So before we can do this, we need to introduce some kind of gathering system/cache/?
A system that makes sure that if there are multiple Consumers, then they utilize the same DOM event.
Summary
Spike to modernize the log-viewer package — one of the oldest in the backoffice — by introducing two new Context API primitives and applying them across every context-consuming file.
Builds on top of #22519 (
Context API: Defer decorator controller creation to hostConnected— which now also alignsUmbControllerHostMixinwith Lit'sReactiveElementpattern). Review that first.New primitives (in
libs/context-api/observe/)@observedFrom(token, selector, { default })— accessor/property decorator that binds the decorated property to an observable slice of a context. Combines consume + observe + assign in one declaration.this.observeContext(token, selector, callback)— method onUmbClassMixin/UmbElementMixinfor side-effect-only observations (no bound value):Thanks to the mixin refactor in #22519, the decorator registers its
UmbContextConsumerControllerdirectly viaaddUmbController— noqueueMicrotaskdeferral, no throwaway wrapper.Migration: log-viewer
Applied to 11 files across all four pattern categories (simple, paged, complex, manual cleanup):
log-search-view.element.tslog-viewer-polling-button.element.tslog-viewer-date-range-selector.element.tslog-viewer-log-level-overview.element.tslog-viewer-log-level-filter-menu.element.tslog-overview-view.element.tslog-viewer-message-templates-overview.element.tslog-viewer-saved-searches-overview.element.tslog-viewer-messages-list.element.tslog-viewer-search-input.element.tslog-viewer-log-types-chart.element.tsCounting only changed regions (git stat): −241 / +99 across the 11 files.
Each file loses the
#ctx/set _ctx/get _ctx/#observe*/ individual@statetriads. Observable state becomes declarative. Side-effect observations (filter → refetch) move tothis.observeContext. Imperative context method calls stay the same.How
@observedFromworksThe decorator wraps
UmbContextConsumerController+element.observe(). A stable per-field observer alias keeps subscriptions tidy across context instance changes. When the selector's observable emitsundefined(common withasObservablePart((data) => data?.items)over nullable state), the decorator falls back to the declareddefault— matching thevalue ?? defaultdefensive pattern consumers previously wrote by hand.Test plan
@observedFromandobserveContextcovering: bind, re-render on emit, default value, late-arriving provider, re-subscription on new provider, unprovide cleanup, default-on-undefined-emission,UmbControllerBasehostConnected-wrapper pathWhat is NOT in this PR (deferred)
Kept scope tight for reviewability. Follow-ups worth picking up:
Log-viewer context-side modernization
logviewer-workspace.context.tscurrently provides itself under a custom token, which tangles data-fetching + UI-state + URL-sync responsibilities. Either make it a plain global context or integrate properly with the workspace base.catch(() => {})withUmbNotificationContexttoast feedback on save/delete failuresUmbRepositoryBaseon the log-viewer repository to align with current conventionssetInterval(...) as unknown as number—window.setIntervalreturnsnumberin browsersNext Context API primitive:
umbContextObserveLit directiveFor render-only cases where you don't need a class field. The log-viewer still has a couple of these — e.g.
log-search-view.element.tsandlog-overview-view.element.tsboth observectx.canShowLogspurely to drive a conditional render:Shape: Lit
AsyncDirectiveimplementingupdate/disconnected/reconnected. Holds the context consumer and subscription internally, keyed on(host, token, selector reference). Cleans up on any of: observable/selector change, part removal, element disconnect.@observedFromis the higher-leverage primitive (usable in render AND derived logic), so the directive is optional sugar — nice to have for the "pure conditional render" pattern.Other small items
@observedFromconsumer cache — per-host-per-token sharing. Four@observedFromfields pointing at the same context currently create fourUmbContextConsumerControllers. Not measurable in practice but easy to collapse.UmbPageResult<T>— the{ items, total }→ exposed getters pattern is repeated in the paged views; worth a helper#d42054) and sizes (20px,10px 20px) should move to--uui-*tokensrole="columnheader"/aria-labelon the messages tablePropertyValueMap<any>→PropertyValueMap<this>; modal manifest in its own file🤖 Generated with Claude Code