Skip to content

fix(bindx-react): fetch and render a parameterised HasMany under its own alias - #159

Open
vparys wants to merge 4 commits into
mainfrom
fix/hasmany-params-alias
Open

vparys wants to merge 4 commits into
mainfrom
fix/hasmany-params-alias

Conversation

@vparys

@vparys vparys commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

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's map(), which wrote them into the plain relation as well as returning the parameterised alias. So:

  • the query fetched the whole relation with the children's fields next to the filtered subquery;
  • at render, HasManyImpl read the field it was given, and EntityHandle.resolveSelectionField prefers the direct plain field over the alias — the list ignored its parameters (filter={{ name: { eq: 'React' } }} rendered JavaScriptReact). 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

  • Collection: 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 / toSelectionMeta carry it. The collector ref exposes HAS_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.
  • Render: HasManyListHandle carries HAS_MANY_ALIAS_RESOLVER, provided by the parent EntityHandle; <HasMany> computes its params alias and reads that list. The resolver declares the alias's view membership (as the fields getter does), so an orderBy-only list keeps showing items added to the relation. For an alias the parent's selection did not fetch (a useEntity definer, params changed since collection) it returns nothing and <HasMany> reads the field as given, as on main. An entity without a selection gets the alias handle only when its data holds the alias (a HasManyDataGrid row); otherwise, as in create mode, it keeps the field as given.
  • Compiler: the field-map format has no params, so the compiler now bails on any parameterised <HasMany>, and on one with spread props (HASMANY_PARAMS, formerly HASMANY_PARAMS_AT_ROOT for roots only) and the runtime pass handles it. Dropped the now unused isSelectionRoot flag.

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. On main it 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 on main.

Remaining

  • Params that change after collection: the new alias is not in the selection until the selection is collected again (fix(bindx-react): collect the selection again when Entity or EntityList children change #156 does that when the children change), so until then the plain list shows.
  • An alias that is in the selection but whose data has not arrived yet (a grown selection before its refetch lands) renders an empty list until it does; same as the fields getter on main for a useEntity definer whose params change.
  • An explicit as alias in a fluent definer (.tags({ as: 'reactTags', filter }, …)) is still re-keyed to the generated alias when merged into a scope; on main it was lost too.

Reading article.tags at all still registers the plain relation with id (SelectionScope.child seeds it), so a filtered <HasMany> still fetches the relation's ids. Making that registration lazy would break plain runtime reads like article.tags.length that the collector does not see; left as is.

Tests

  • tests/react/relations/hasMany/filteredHasManyFetchesUnfilteredRelation.test.tsx: children fields only under the alias (plain relation is id only); a filtered list renders filtered; sibling <HasMany>s with different parameters each render their own list (these three fail on main); an orderBy-only list keeps showing an added item; a createComponent with a filtered <HasMany> renders the filtered list; an entity handle without a selection (as HasManyDataGrid builds 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.
  • Compiler tests updated for the wider bail, plus the spread case.

bun run test: 2531 pass. bun run typecheck clean.

…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 matej21 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.tsx and packages/bindx-compiler/tests: 265 pass. bun run typecheck (tsc --build): clean.
  • The head merges cleanly onto current main (6a1f600). CI on 1b030f7 is 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_RESOLVER and HasManyAliasResolver are new exports of @contember/bindx. The compiler's BailoutReason renames HASMANY_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.
  • declareAliasViewMembership writes to the store during render. It is idempotent and does not notify, the same pattern as the existing fields getter. 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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Filtered HasMany also fetches the unfiltered relation

2 participants