[cowboyd]

Elements participate in several overlapping scopes: host ownership, layout behavior, focus state, event handling, and rendering policy. Passing every capability through every operation would make element authors and framework adapters carry plumbing that has nothing to do with their component logic. Nodes with inherited contexts provide ambient, subtree-scoped capabilities while still allowing a descendant to override any individual concern.

The tree at this layer records semantic attachment, not visual order. That keeps the primitive small and allows the ordered host tree introduced later to define presentation without turning every middleware concern into a layout concern.

import { Node, createContext } from './core.ts';

const root = new Node();
const child = new Node(root);
const color = createContext<string>('color');

color.set(root, 'cyan');
color.get(child); // => "cyan"

This PR adds the minimal Node and Context classes, parent references, child collections, and prototype-inherited context storage inside the UI core. createContext() remains as the ergonomic context factory, with tests covering inheritance, overrides, local ownership, defaults, and null-terminated lookup.

[pkg-pr-new]

• @examples/basic

  npm i https://pkg.pr.new/bombshell-dev/ui/@clack/ui@17
  

commit: ad4632c

[43081j]

FYI this pattern can get pretty slow since it means repeated property access in the hot path.

V8 shouldn't deopt it at least if every node has the same shape. But you are probably better off storing the state separately.

E.g. in a WeakMap<Node, Context>.

Similarly, it'll end up in dictionary mode in V8 given the context object will be inconsistently shaped. We really should use a Map instead of a plain object if we can.

[cowboyd]

If we don't use an object here, then we'll have to implement the prototype chain ourselves in Context.get()

[43081j]

I'm a bit lost, can you explain more what you mean?

This is just a properties bag attached to the node right now. I'm suggesting don't attach it to the node, just relate them through a weak map instead.

If this is actually meant to be context: Context right now, the same applies. WeakMap<Node, Context>

[cowboyd]

It's so that contextual values are inherited from one node to the next in the tree.

https://github.com/bombshell-dev/ui/pull/17/changes#diff-5ccb295a69b0308bcf422c94e309ec4a592fe62277dc6b6702ea0bc8adda412eR81

We can use a Map as the underlying storage mechanism, but then we'd have to implement the lookup ourselves.

[43081j]

it is clearer now that you moved to a class, you're talking about this inheritance:
https://github.com/bombshell-dev/ui/pull/17/changes#diff-3b2a6d756e64248f2ab7b374b867eadb4e3bae17231db7308752d43bd113372dR8

you can still have that without storing it on the node, though.

const contexts = new WeakMap<Node, Context>();

// elsewhere, in the Node constructor
const parentContext = parent ? contexts.get(parent) : null;
contexts.set(this, Object.create(parentContext, whatever));

// anywhere you need a context
contexts.get(node); // Context or undefined

it is more expensive to repeatedly be accessing properties of nodes during traversal than it is to look them up in a weak map, basically.

[dreyfus92]

i don't think the WeakMap buys us anything here. your idea still calls Object.create(parentContext). the inheritance mechanism is identical, it's just parked behind a WeakMap.get first. so it's the same prototype walk plus a hash lookup, not fewer property accesses.

same with Map it has no prototype chain, so Context.get would have to loop up parent by hand.

the one thing WeakMap does give us is that contexts stops being public on Node. if that's the concern I'm on board 👀

[43081j]

You've misunderstood a bit because this wasn't about the inheritance at all. It was purely about the property access:

FYI this pattern can get pretty slow since it means repeated property access in the hot path

the WeakMap absolutely would have bought us a perf bump. however, since Node is now a class, i suspect it means v8 can optimise the shape into the same hidden class across the board.

happy to resolve this because of the move to using a class.

[43081j]

re my other comment, this is a slow path that would be made a lot faster if it was just a map. this.contexts.has(name) etc

[dreyfus92]

a Map has no prototype chain, so you'd hand-roll the parent walk in Context.get which is O(depth) loop instead of one engine-native lookup. i don't think is worth it now.

[43081j]

why would we hand roll any kind of walk?

assuming every node has a context, and the context is a Map, its just a plain old .get(name) or .has(name) like in my comment above.

in the current code and my proposed code there's no concept of a prototype chain. its a flat hasOwn of a single object.

EDIT: ok i understand now. this manual inheritance stuff is a pain. contexts isn't a regular object, its the prototype chain from before. so we can't replace it with a Map elsewhere (this usage absolutely could use a map) thanks to that... i need to think about this as its forcing us into obvious perf bottlenecks.

[cowboyd]

@43081j We can definitely give all those a try. I also tried to make the middleware (#18 ) class-based as well and use super.method(...args) instead of an explicit next(...args) parameter, but I couldn't get it quite to work so I moved on. (might be worth revisiting)

For performance concerns, we should get a perf suite set up so that we don't introduce any regressions. I don't know how folks are feeling about CodSpeed which we are using for tty?

[43081j]

i wouldn't spend much time on setting up a benchmark suite yet as we're barely at the architecture level.

we're better off tackling that when we've actually settled on a design.

[dreyfus92]

i agree with james, we shouldn't be worrying about perf issues rn.

[43081j]

tree-node...tree-node-jamesified

here you go, also messaged it in discord 👍

[natemoo-re]

Works for me! This seems to match @43081j's implementation quite literally. 💜

[dreyfus92]

let's keep this rollin' bois 🤘🏻