RFC#1200: Add makeContext: provide/consume#21450
Open
NullVoxPopuli wants to merge 1 commit into
Open
Conversation
* Prototype RFC #1154: getScope/addToScope render-tree primitives Adds a public, always-on render-tree scope tracker that powers the component-tree provide/consume pattern that the Ember community has been asking for in RFC #975 / #1154. Public API (exported from @ember/renderer): import { getScope, addToScope, type Scope } from '@ember/renderer'; // Inside any code that runs during rendering: let scope = getScope(); // current scope, or undefined addToScope({ key: 'theme', value: 'dark' }); // Walk up the render tree: for (let entry of scope.entries) { ... } Implementation notes: - `RenderScopeTracker` lives in @glimmer/runtime parallel to `DebugRenderTree`, but is always-on because this is part of the public surface area (not a debug-only tool). - Component lifecycle wires the tracker into: VM_CREATE_COMPONENT_OP -> push scope before manager.create() so user-land constructors can call addToScope against their own scope. VM_DID_RENDER_LAYOUT_OP -> pop scope on initial render and on every updating frame. Updating opcodes (RenderScopeUpdateOpcode / RenderScopeExitOpcode) re-push and pop on re-renders so descendant scope reads stay correct. - The Scope's `entries` iterator walks the current node's own additions newest-first, then up through each ancestor. This is the exact shape a userland `consume(key)` needs to find the nearest provider. - Begin/commit reset the stack and the module-level "active tracker" pointer, so getScope() correctly returns undefined outside of render. Userland provide/consume is included as an integration test (in packages/@ember/-internals/glimmer/tests/integration/render-tree-scope-test.ts): a `<Reader/>` nested inside multiple `<Provide/>` components consumes the nearest provider's value, exactly matching the "How we teach this" example in RFC #1154. Scope of this prototype: components only. Helpers, modifiers, and plain-curly functions are not yet wired, which matches the immediate provide/consume use case. Extending to other invokables is straightforward once the shape lands -- the tracker doesn't care what the bucket is. Refs: - emberjs/rfcs#1154 - emberjs/rfcs#975 - https://github.com/customerio/ember-provide-consume-context (prior art) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix lint: prettier format + docs coverage for getScope/addToScope - Run prettier on render-scope.ts. - Register getScope and addToScope in tests/docs/expected.cjs so the docs-coverage test recognises the new @ember/renderer exports. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add makeContext(): user-facing provide/consume per NullVoxPopuli's RFC #1154 comments NVP proposed in emberjs/rfcs#1154 (comment) that the actual user-facing primitive should be `makeContext`: const foo = makeContext(Foo) <foo.Provide> {{#let (foo.consume) as |f|}}{{f.bar}}{{/let}} </foo.Provide> {{ (foo.consume) }} <-- throws This commit pivots the public API in @ember/renderer from the lower-level getScope/addToScope primitives (which stay as internal infrastructure) to the higher-level makeContext returning `{ Provide, consume }`. Behavior matches NVP's clarifications: - consume() throws when no <Provide> is found in the render tree. - consume() throws when called outside a render (the scope is render-time only; an undefined scope is never legitimate for context). - The value returned by the factory is not itself tracked, but @Tracked state on it remains reactive -- consumers re-render when those fields change. - Two forms supported, per NVP's example and rtablada's extension: makeContext(Klass) // each <Provide> calls `new Klass()` makeContext(() => value) // each <Provide> calls the factory Detected via a Function.prototype.toString sniff (`/^class[\s{]/`). Implementation notes: - `<Provide>` is built on the same internal-component infrastructure as Input / Textarea / LinkTo (lib/components/internal.ts + `opaquify`), so it ships inside ember-source without taking a dep on @glimmer/component. - Each `<Provide>` constructor instantiates the factory and pushes [key, value] onto the current render-tree scope; consume walks scope.entries looking for a matching key. The closure-captured `key` identity isolates contexts from each other. - The `<Provide>` template is the static `{{yield}}`, precompiled once and shared across all Provide classes. Tests (packages/@ember/-internals/glimmer/tests/integration/render-tree-scope-test.ts) cover the five things real consumers care about: throws outside render, throws with no provider, nearest-provider lookup, factory form, and @tracked-reactivity through the provided value. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix reactive-context test: capture instance via factory, not empty Capture component The previous test used a Capture component with an empty template to grab the instance via its constructor. An empty template renders as `<!---->` in the DOM, which polluted assertHTML('0') -> actual was `<!---->0`. Move the capture into the factory closure itself -- the factory runs exactly once per <Provide>, so it's a clean place to grab the instance without adding any DOM artifacts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add <Provide @value> + port ember-provide-consume-context test scenarios Extends `<Provide>` with an optional `@value` arg (matching rtablada's RFC #1154 example) and ports the substantive cases from customerio/ember-provide-consume-context's built-in-components-test.ts. @value support: - `args.named.value` is stored as a lazy `read()` thunk in the scope entry. `valueForRef` consumes the tracking tag when called inside the consumer's tracking frame, so consumers re-render automatically when the arg updates. - When `@value` is not passed, the factory runs once per <Provide> and the cached result is returned (preserves identity across re-renders, which downstream code -- ref tracking, caching -- relies on). The scope-entry shape changes from `[key, value]` to a typed `{ key, read }` record (with an `isContextEntry` guard) so that future extensions don't have to overload the array form. Tests ported / adapted (in the new "behavior ported from ember-provide-consume-context" module): - a consumer can read context - a consumer reads from the closest provider - consumer's value updates when @value changes - a consumer can't access a context it isn't nested in - sibling Provides with the same context do not bleed - consumer is reactive across an {{#if}} that toggles it on and off - a conditional <Provide> tears down and re-instates correctly - a conditional sibling <Provide> does not override an outer one - multiple distinct contexts can be nested - @Tracked state on a factory-provided class instance is reactive - consumer at component-instance init time sees the nearest provider - factory-provided value is stable across the same Provide re-render EPCC tests that did NOT port: - "reading a context that does not exist returns undefined" -- the makeContext API throws instead, per NVP's "reduce harm" clarification. Already covered by the "consume() throws when no <Provide>" test. - @provide / @consume decorator tests -- decorators are a separate API paradigm not in scope for the makeContext primitive. - test-support helpers (`setupRenderWrapper`, `provide` in beforeEach) -- test-support is a separate concern that should be addressed once the primary API lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix template build: inline multi-line precompileTemplate strings babel-plugin-ember-template-compilation requires the first argument to precompileTemplate to be a literal string. The .join('\n') array form broke the build for the ported EPCC test cases. Switch them to template literals. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * makeContext: use isNewable check; add 7 gap tests Replace the `/^class[\s{]/` toString sniff with the prototype-based `isNewable` pattern lifted from ember-primitives (ember-primitives/src/utils.ts): proto !== undefined && proto.constructor === fn Arrow functions have no `prototype` and fail this check; classes (and old-style constructor functions) pass. Robust under transpilation, where the toString check would silently regress. Adds two new test modules covering the previously-identified gaps: extra-coverage: - class-form (`makeContext(SomeClass)`) is actually invoked with `new` (guarded by an in-constructor `new.target === undefined` check, so any regression to plain invocation fails the test) - consume() works inside a plain function helper (`defineSimpleHelper`) - consume() works inside a modifier (`defineSimpleModifier`) - explicit `@value={{undefined}}` provides undefined (does NOT throw "no provider") - explicit `@value={{null}}` provides null - multiple consume() calls in the same template return the same instance cross-renderComponent isolation: - two independent `renderComponent` calls into separate sub-elements do not share scope state: a <Provide> in one tree is invisible from the other Engine / `{{outlet}}` boundaries remain explicitly out of scope -- they need design discussion, not just a test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * makeContext: pin down modifier-install scope limitation (not a regression) The "consume() inside modifier" test asserted the wrong thing. Modifier install runs during `transaction.commit()`, which fires AFTER the render frame has popped its scope stack -- so consume() inside a modifier callback legitimately throws "outside of rendering". Rewrite the test to assert that throw and document it as a known limitation. This pins down the current behavior so a future fix (e.g. re-pushing the enclosing component's scope for the duration of modifier install) doesn't break silently. RFC #1154 motivates "all invokables" -- modifier support is a follow-up worth its own design discussion, since it interacts with the transaction model. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * makeContext: drop the factory arg, provide value via <Provide @value> `makeContext` no longer takes a class/factory. It takes a type parameter only (`makeContext<T>()`) and the value is supplied at render time through `<Provide @value={{...}}>`. This removes the dual class/factory forms (and the `isNewable` detection + `ContextFactory` type) in favor of a single, explicit way to provide a value. - `consume()` still throws outside rendering and when no provider exists. - Omitting `@value` (or passing undefined/null) provides that value rather than throwing -- the provider is in the tree, it just has no value. - The `@value` binding stays reactive via `valueForRef`. Rewrote the integration suite to the `@value` API and added an explicit smoke-test module. All 23 tests pass in headless Chrome; tsc, eslint, prettier, and docs coverage are clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Re-export makeContext from @ember/helper (not @ember/renderer) makeContext is a helper-style API (it returns a `consume` usable as a template helper), so it belongs alongside the other helpers. Move the public export from @ember/renderer to @ember/helper and update the `@module`/`@for`/import-example docs accordingly. Add a type smoke test in type-tests/@ember/helper-tests.ts that pins the generic-only signature: `makeContext<T>()` returns `Context<T>`, `consume()` returns `T`, and passing a class is now a type error (`@ts-expect-error makeContext(Theme)`). type-check:internals, type-check:types, eslint, prettier, and docs coverage are clean; the 23 makeContext browser tests still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add end-to-end makeContext smoke test to the app scenarios Adds `make-context-test.gjs` to the basic smoke-test app (run across the classic / embroider-webpack / embroider-vite scenarios). Unlike the @glimmer/runtime QUnit tests, this exercises makeContext through the published `@ember/helper` export in a real built app: - provide a value via `<Provide @value>` and consume it - nearest-provider lookup with nested `<Provide>` - consumer re-renders when `@value` changes (tracked) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Simplify render-scope: context-specific provide/lookup, drop getScope/addToScope makeContext is the only consumer of the render-tree scope, and the only public API in this PR -- so the generic `getScope`/`addToScope` surface (the `RenderScope` view object, its `entries` iterable, the lazy view caching, and the `ContextEntry` type guard in make-context) was more machinery than the feature needs. Replace it with two context-specific helpers in @glimmer/runtime: provideRenderContext(key, read) // <Provide> stores key -> lazy read lookupRenderContext(key) // consume() walks up for the nearest // undefined = outside rendering // null = no provider // fn = nearest read Each render node now holds a lazily-allocated `Map<key, read>` instead of an untyped entry array, and `consume()` is a direct walk-up + read rather than iterating an `unknown` entry stream and type-guarding each item. The RenderScopeTracker lifecycle (create/enter/exit/willDestroy + the updating opcodes) is unchanged, as is all observable behavior. The public `RenderScope` interface and the `getCurrentScope`/ `addToCurrentScope` members are removed from @glimmer/interfaces; the tracker interface now exposes only the render-node lifecycle. tsc clean; all 23 makeContext browser tests still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * makeContext: use `consume` as the context identity, drop the empty-object key The render-scope lookup needs a stable, unique-per-context identity token. That was a freshly-allocated `{}`, but `consume` already is one: it is created once per `makeContext()` call and is in scope for both the `consume()` reader and the `<Provide>` constructor. Reusing it as the Map key removes the extra object (and the "why is there an empty object?" question) with no behavior change. All 23 makeContext browser tests still pass; tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Trim the render-scope tracker to only the methods that are reachable Audited every method against what actually runs: - Drop `commit()` -- it only called `reset()`, which `begin()` already does at the start of the next transaction. Removed its call in environment.ts. - Drop `willDestroy()` (and the `associateDestroyable` + `registerDestructor` wiring in the create opcode). `lookup` only ever sees nodes via the live stack, and a destroyed component is never re-entered, so this was pure eager cleanup -- the `nodes` WeakMap collects on GC regardless. - Drop the `isRendering` getter and the private `reset()`; fold both into `lookup` (returns `undefined` when there is no current frame) and an inline loop in `begin()`. - Pare the `RenderScopeTracker` interface in @glimmer/interfaces down to the three lifecycle methods the opcodes actually call (`create`/`enter`/`exit`). What's left is the irreducible set: begin (error recovery), create/enter/exit (stack lifecycle, proven load-bearing), and provide/lookup (the two real ops). All 23 makeContext browser tests pass; tsc clean. Net -66 lines. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix stale doc comments: @for tag, create-opcode reference, test coverage list Comment-only corrections surfaced by an audit after the API/internals changes landed: - make-context.ts: `@for @ember/renderer` -> `@ember/helper` (makeContext is exported from @ember/helper now; matches the canonical block in @ember/helper/index.ts). - component.ts: the VM_DID_RENDER_LAYOUT_OP exit comment referenced `VM_GET_COMPONENT_SELF_OP`; the matching `create()` is in `VM_CREATE_COMPONENT_OP`. - render-tree-scope-test.ts: note the "omitting @value" case in the extra-coverage suite summary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📊 Size reportTarball size — dist/dev 0.4%↑
dist/prod 0.5%↑
smoke-tests/v2-app-hello-world-template/dist 0.3%↑
🤖 This report was automatically generated by wyvox/pkg-size |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Update:
needs RFC (I'm going to re-submit RFC#1154 with some updates from things I discover here
I also want to make sure that the strategy here enables us (or just straight up implements since this is a draft PR), the ability to getOwner() within helpers and such (no arguments passed to getOwner, but getOwner is "render aware" (and thus would not work after an await).
Example implementation that builds on top of this PR is only 3 lines: https://github.com/NullVoxPopuli/ember.js/pull/16/changes#diff-875d40027d0d5b381dba626bd5e26a36e7cd5d1e5ffa180df4a0183be09b9c65R59
example getOwner
at the moment, I'm not really sure where to put these APIs, so I put them on
@ember/helperfor now.API is basically this (taken from one of the tests):