Repository navigation
Conversation
…own alias A <HasMany> with filter, orderBy, limit or offset collected its children through the collector's map(), which wrote them into the plain relation as well. The query therefore carried the whole relation with the children's fields next to the parameterised subquery, and at render the HasMany read the plain relation: the list it showed ignored its parameters. SelectionScope now keeps a has-many read with parameters as its own child keyed by the params alias, and the collector hands <HasMany> an item over that child, in every collection path including a component's implicit one. At render, HasMany resolves its params alias through the parent entity (HAS_MANY_ALIAS_RESOLVER) and reads the data fetched under it. The compiled field-map format has no params, so the compiler now bails on any parameterised <HasMany> (HASMANY_PARAMS), not only in an <Entity> root. Fixes #158
…t, and fall back when unfetched The resolver now declares the alias's view membership like the fields getter does, so an orderBy-only <HasMany> keeps showing items added to the relation. It returns nothing for an alias the parent's selection did not fetch, and <HasMany> then reads the field as given. The compiler also bails on a <HasMany> with spread props, which may carry params, and a has-many child merged with totalCount keeps it.
…ection HasManyDataGrid rows are entity handles without a selection, so the resolver could not find the alias and the HasMany read the plain relation, which now carries only ids. The resolver takes the params, computes the alias itself and, without a selection, returns the alias handle with its view membership derived from the params.
…o data under the alias In create mode, and on any entity without a selection whose alias was never fetched, the alias view fell back to the plain relation's server data but was declared partial, so items added to the relation never showed in a filtered or limited list. Without a selection the resolver now returns the alias handle only when the entity's data holds the alias.
matej21
left a comment
There was a problem hiding this comment.
Review round 1: changes
TLDR: The fix works for literal params: no overfetch, and the list renders filtered. But when params change at runtime (a filter driven by state), the subtree now crashes with UnfetchedFieldError. On main it rendered the unfiltered list. That case needs handling and a test before merge. Two minor items and one behaviour decision for the maintainer follow.
Reviewer: review agent (claude-opus-5-5). Commit reviewed: 1b030f7.
Verified
- PR tests,
tests/selectionCollection.test.tsxandpackages/bindx-compiler/tests: 265 pass.bun run typecheck(tsc --build): clean. - The head merges cleanly onto current
main(6a1f600). CI on1b030f7is green. - Literal params in an
<Entity>root and in a component: the query carries the children's fields only under the alias, and the list renders filtered. Confirmed with a probe; it matches the PR tests.
Findings
1. Major: params that change after collection crash the subtree (regression).
<Entity> collects its selection once per by, so a new params alias is not in the selection. resolveHasManyAlias returns undefined, and <HasMany> falls back to the plain field. With this PR that field holds only id, so the children's first field read throws.
function Probe() {
const [name, setName] = useState('React')
return (
<Entity entity={schema.Article} by={{ id: 'article-1' }}>
{article => (
<HasMany field={article.tags} filter={{ name: { eq: name } }}>
{tag => <Field field={tag.name} />}
</HasMany>
)}
</Entity>
)
}
// setName('JavaScript') →
// UnfetchedFieldError: Accessing unfetched field "name" on entity Tag(tag-1).On the PR base (6f02df1) the same component renders JavaScriptReact: wrong, but no crash. The description's "Remaining" note says "until then the plain list shows". That is not what happens: it throws. A search box over a has-many is a common shape, so this should not ship as is.
With #156 merged on top (local merge, not pushed), the new alias is collected and refetched. The store then holds it (getPresentationHasManyOrderedIds(…, 'tags_bss18h') → ["tag-1"]), and the children render tag-1. But <Field field={tag.name}> stays empty, even after a forced parent re-render. My hypothesis, not verified: the item's Field first rendered against the plain-relation fallback, when the Tag:tag-1 snapshot held only id. The later fill from the alias data does not reach the memoised Field. So merging #156 first does not close this on its own either.
Suggested direction (the contributor's call): handle the unfetched-alias case in <HasMany> itself. Show an empty or loading list until the alias is in the selection, instead of falling back to a plain relation that no longer carries the children's fields. Add a test that changes the params at runtime, alone and together with #156.
2. Minor: params computed from a scalar prop in a component use the collection mock.
createComponent().props<{ name: string }>().render(({ article, name }) => <HasMany field={article.tags} filter={{ name: { eq: name } }}>…): during implicit collection, name is createScalarPropMock(), a function proxy that JSON.stringify drops. The query therefore gets an extra subquery, tags_3n5d80 with filter: { name: {} }. At render the real alias differs, so the list falls back to the plain relation and shows everything. The visible result equals main, but a mock-derived filter now goes to the server, which main dropped. I did not verify what Contember does with an empty condition. With .mock({ name: 'x' }) the aliases still differ at runtime. Suggest either skipping alias collection when params contain a collection mock, or documenting this as a limitation with a test that pins the behaviour.
3. Minor: new type escapes.
HasMany.tsx adds three as unknown as casts, for the resolver lookup, the aliasList cast and the HAS_MANY_PARAMS_COLLECTOR lookup. SelectionScope.hasManyChild adds this.relationMeta.get(alias)!. The repo's rules say no as. Declaring [HAS_MANY_ALIAS_RESOLVER]?: HasManyAliasResolver on HasManyRef, and the collector symbol on the collector ref type, would remove the casts. The non-null assertion can become a guarded branch.
4. Decision for the maintainer (not a defect): added items in filtered or limited lists.
As the description says, an item added through the outer field no longer appears in a filter, limit or offset <HasMany> until it is persisted and refetched. This follows the store's 'partial' view semantics, and orderBy-only lists still show it. It is a visible UX change, for example a "latest 5 comments" list with an add button. Worth an explicit yes.
5. Info.
- Public API:
HAS_MANY_ALIAS_RESOLVERandHasManyAliasResolverare new exports of@contember/bindx. The compiler'sBailoutReasonrenamesHASMANY_PARAMS_AT_ROOT→HASMANY_PARAMS. Both are acceptable under the repo's no-BC policy. - The compiler now bails on every parameterised
<HasMany>, and on any<HasMany>with spread props. That is sound, but fewer components compile; components with such lists fall back to the runtime pass. declareAliasViewMembershipwrites to the store during render. It is idempotent and does not notify, the same pattern as the existingfieldsgetter. Fine.
Missing tests
- Params changing at runtime (finding 1).
- A component whose
<HasMany>params come from a scalar prop (finding 2).
Decision table
The PR comes from an external contributor and has no brief with decisions to check. N/A.
— review agent (claude-opus-5-5)
Fixes #158. Also item 2 of
docs/issues/036-jsx-conditional-underselection-and-overfetch.md.Problem
<HasMany filter|orderBy|limit|offset>collected its children through the collector'smap(), which wrote them into the plain relation as well as returning the parameterised alias. So:HasManyImplread the field it was given, andEntityHandle.resolveSelectionFieldprefers the direct plain field over the alias — the list ignored its parameters (filter={{ name: { eq: 'React' } }}renderedJavaScriptReact). The aliased subquery was fetched and never read.Inside a component the implicit collection path dropped the params alias altogether, so there only the plain relation existed.
Change
SelectionScope.hasManyChild(fieldName, params)keeps a parameterised has-many as its own child keyed by the params alias, next to the plain one;merge/mergeFromSelectionMeta/toSelectionMetacarry it. The collector ref exposesHAS_MANY_PARAMS_COLLECTOR, and<HasMany>with parameters collects its children through it — in every path (an<Entity>root, a component's implicit pass, nested). Without parameters nothing changes.HasManyListHandlecarriesHAS_MANY_ALIAS_RESOLVER, provided by the parentEntityHandle;<HasMany>computes its params alias and reads that list. The resolver declares the alias's view membership (as thefieldsgetter does), so an orderBy-only list keeps showing items added to the relation. For an alias the parent's selection did not fetch (auseEntitydefiner, params changed since collection) it returns nothing and<HasMany>reads the field as given, as onmain. An entity without a selection gets the alias handle only when its data holds the alias (aHasManyDataGridrow); otherwise, as in create mode, it keeps the field as given.<HasMany>, and on one with spread props (HASMANY_PARAMS, formerlyHASMANY_PARAMS_AT_ROOTfor roots only) and the runtime pass handles it. Dropped the now unusedisSelectionRootflag.Behaviour change
A filtered, limited or offset
<HasMany>now shows the filtered list, so an item added through the outer field (article.tags.add(…)) does not appear in it until it is persisted and refetched — the store's'partial'view semantics. Onmainit appeared only because the whole unfiltered list was rendered. An orderBy-only list keeps showing added items. In create mode nothing has been fetched, so a parameterised<HasMany>shows the relation as given, including added items, as onmain.Remaining
fieldsgetter onmainfor auseEntitydefiner whose params change.asalias in a fluent definer (.tags({ as: 'reactTags', filter }, …)) is still re-keyed to the generated alias when merged into a scope; onmainit was lost too.Reading
article.tagsat all still registers the plain relation withid(SelectionScope.childseeds it), so a filtered<HasMany>still fetches the relation's ids. Making that registration lazy would break plain runtime reads likearticle.tags.lengththat the collector does not see; left as is.Tests
tests/react/relations/hasMany/filteredHasManyFetchesUnfilteredRelation.test.tsx: children fields only under the alias (plain relation isidonly); a filtered list renders filtered; sibling<HasMany>s with different parameters each render their own list (these three fail onmain); an orderBy-only list keeps showing an added item; acreateComponentwith a filtered<HasMany>renders the filtered list; an entity handle without a selection (asHasManyDataGridbuilds its rows) renders the filtered list; in create mode a filtered and a limited list show an item added through the outer field.tests/selectionCollection.test.tsx: the inline<HasMany orderBy>case now looks the relation up under its alias.bun run test: 2531 pass.bun run typecheckclean.