Skip to content

Commit 15ca618

Browse files
Copilotaarne
andauthored
Fix GraphQL resolver for JSONObject/[JSONObject] fields instead of hiding behind standalone fallback (#130)
* Initial plan * Fix GraphQL replay for wildcard and bare leaf field selections - Handle wildcard paths (e.g. "legs.*") in mergeObservedSelection so the schema observer sees the full field data - Add resolveSelectionsAgainstSchema to expand wildcards and bare leaf selections on object-typed fields in the generated GraphQL query - Remove disable: ["graphql"] from both sparse fieldset test scenarios Co-authored-by: aarne <82001+aarne@users.noreply.github.com> * Fix field selection semantics and add harness regression tests - Wildcard (field.*) now selects only scalar sub-fields, not object-typed - Bare leaf (field) uses JSONObject schema replacement with standalone execution mode for correct pass-through behavior - Add buildScalarOnlySelectionTreeFromType for wildcard expansion - Add collectFieldsRequiringJSONObject and replaceFieldTypesWithJSONObject for per-scenario schema modification - Add field-selection-harness.test.ts with 10 tests documenting semantics - Include test/utils/*.test.ts in the test script Co-authored-by: aarne <82001+aarne@users.noreply.github.com> * Use proper GraphQL AST types instead of any in harness helpers Co-authored-by: aarne <82001+aarne@users.noreply.github.com> * Initial plan Co-authored-by: aarne <82001+aarne@users.noreply.github.com> * Fix GraphQL handler for JSONObject fields; remove standalone fallback Fix three bugs in ExecutionTree.response() for scalar-typed fields: 1. Array passthrough ([JSONObject]): skip shadow tree creation when the field is a pure passthrough with no element-level mappings. Shadow trees caused collectOutput() to materialise the wrong fields from the root bridge. 2. Shadow fallback ([JSONObject] in array-mapped output): return plain data instead of wrapping in shadow trees when scalar=true. 3. Structured sub-field (JSONObject): eagerly resolve nested field via resolveNestedField() instead of returning the ExecutionTree itself, which caused collectOutput() to materialise all output fields instead of just the sub-field. Remove the standalone execution workaround from the test harness. Add non-array object selection regression tests and focused GraphQL unit tests for JSONObject / [JSONObject] fields. Co-authored-by: aarne <82001+aarne@users.noreply.github.com> * Docs --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: aarne <82001+aarne@users.noreply.github.com> Co-authored-by: Aarne Laur <aarne.laur@gmail.com>
1 parent e16ef7d commit 15ca618

8 files changed

Lines changed: 1123 additions & 11 deletions

File tree

packages/bridge-core/src/ExecutionTree.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1751,7 +1751,30 @@ export class ExecutionTree implements TreeContext {
17511751
return response;
17521752
}
17531753

1754-
// Array: create shadow trees for per-element resolution
1754+
// Array: create shadow trees for per-element resolution.
1755+
// However, when the field is a scalar type (e.g. [JSONObject]) and
1756+
// the array is a pure passthrough (no element-level field mappings),
1757+
// GraphQL won't call sub-field resolvers so shadow trees are
1758+
// unnecessary — return the plain resolved array directly.
1759+
if (scalar) {
1760+
const { type, field } = this.trunk;
1761+
const hasElementWires = this.bridge?.wires.some(
1762+
(w) =>
1763+
"from" in w &&
1764+
((w.from as NodeRef).element === true ||
1765+
this.isElementScopedTrunk(w.from as NodeRef) ||
1766+
w.to.element === true) &&
1767+
w.to.module === SELF_MODULE &&
1768+
w.to.type === type &&
1769+
w.to.field === field &&
1770+
w.to.path.length > cleanPath.length &&
1771+
cleanPath.every((seg, i) => w.to.path[i] === seg),
1772+
);
1773+
if (!hasElementWires) {
1774+
return response;
1775+
}
1776+
}
1777+
17551778
const resolved = await response;
17561779
if (resolved == null || !Array.isArray(resolved)) return resolved;
17571780
const arrayPathKey = cleanPath.join(".");
@@ -1820,6 +1843,12 @@ export class ExecutionTree implements TreeContext {
18201843
if (fieldName !== undefined && fieldName in elementData) {
18211844
const value = (elementData as Record<string, any>)[fieldName];
18221845
if (array && Array.isArray(value)) {
1846+
// Nested array: when the field is a scalar type (e.g. [JSONObject])
1847+
// GraphQL won't call sub-field resolvers, so return the plain
1848+
// data directly instead of wrapping in shadow trees.
1849+
if (scalar) {
1850+
return value;
1851+
}
18231852
// Nested array: wrap items in shadow trees so they can
18241853
// resolve their own fields via this same fallback path.
18251854
return value.map((item: any) => {
@@ -1833,6 +1862,13 @@ export class ExecutionTree implements TreeContext {
18331862
}
18341863
}
18351864

1865+
// Scalar sub-field fallback: when the GraphQL schema declares this
1866+
// field as a scalar type (e.g. JSONObject), sub-field resolvers won't
1867+
// fire, so we must eagerly materialise the sub-field from deeper wires.
1868+
if (scalar && cleanPath.length > 0) {
1869+
return this.resolveNestedField(cleanPath);
1870+
}
1871+
18361872
// Return self to trigger downstream resolvers
18371873
return this;
18381874
}
Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
1+
/**
2+
* Tests for JSONObject and [JSONObject] field handling in bridgeTransform.
3+
*
4+
* When a field is typed as JSONObject (scalar) in the schema, the bridge
5+
* engine must eagerly materialise its output instead of deferring to
6+
* sub-field resolvers. This applies to both:
7+
* - `legs: JSONObject` — single object passthrough
8+
* - `legs: [JSONObject]` — array of objects passthrough
9+
*/
10+
import { buildHTTPExecutor } from "@graphql-tools/executor-http";
11+
import { parse } from "graphql";
12+
import assert from "node:assert/strict";
13+
import { describe, test } from "node:test";
14+
import { parseBridgeFormat as parseBridge } from "@stackables/bridge-parser";
15+
import { createGateway } from "./utils/gateway.ts";
16+
import { bridge } from "@stackables/bridge-core";
17+
18+
describe("bridgeTransform: JSONObject field passthrough", () => {
19+
test("legs: JSONObject — single object passthrough via wire", async () => {
20+
const typeDefs = /* GraphQL */ `
21+
scalar JSONObject
22+
type Query {
23+
trip(id: Int): TripResult
24+
}
25+
type TripResult {
26+
id: Int
27+
legs: JSONObject
28+
}
29+
`;
30+
31+
const bridgeText = bridge`
32+
version 1.5
33+
bridge Query.trip {
34+
with input as i
35+
with api as a
36+
with output as o
37+
38+
a.id <- i.id
39+
40+
o.id <- a.id
41+
o.legs <- a.legs
42+
}
43+
`;
44+
45+
const instructions = parseBridge(bridgeText);
46+
const gateway = createGateway(typeDefs, instructions, {
47+
tools: {
48+
api: async (p: any) => ({
49+
id: p.id,
50+
legs: { duration: "2h", distance: 150 },
51+
}),
52+
},
53+
});
54+
const executor = buildHTTPExecutor({ fetch: gateway.fetch as any });
55+
const result: any = await executor({
56+
document: parse(`{ trip(id: 42) { id legs } }`),
57+
});
58+
59+
assert.deepStrictEqual(result.data.trip, {
60+
id: 42,
61+
legs: { duration: "2h", distance: 150 },
62+
});
63+
});
64+
65+
test("legs: [JSONObject] — array of objects passthrough via wire", async () => {
66+
const typeDefs = /* GraphQL */ `
67+
scalar JSONObject
68+
type Query {
69+
trip(id: Int): TripResult2
70+
}
71+
type TripResult2 {
72+
id: Int
73+
legs: [JSONObject]
74+
}
75+
`;
76+
77+
const bridgeText = bridge`
78+
version 1.5
79+
bridge Query.trip {
80+
with input as i
81+
with api as a
82+
with output as o
83+
84+
a.id <- i.id
85+
86+
o.id <- a.id
87+
o.legs <- a.legs
88+
}
89+
`;
90+
91+
const instructions = parseBridge(bridgeText);
92+
const gateway = createGateway(typeDefs, instructions, {
93+
tools: {
94+
api: async (p: any) => ({
95+
id: p.id,
96+
legs: [{ name: "L1" }, { name: "L2" }],
97+
}),
98+
},
99+
});
100+
const executor = buildHTTPExecutor({ fetch: gateway.fetch as any });
101+
const result: any = await executor({
102+
document: parse(`{ trip(id: 42) { id legs } }`),
103+
});
104+
105+
assert.deepStrictEqual(result.data.trip, {
106+
id: 42,
107+
legs: [{ name: "L1" }, { name: "L2" }],
108+
});
109+
});
110+
111+
test("legs: JSONObject — structured output (not passthrough)", async () => {
112+
const typeDefs = /* GraphQL */ `
113+
scalar JSONObject
114+
type Query {
115+
trip(id: Int): TripResult3
116+
}
117+
type TripResult3 {
118+
id: Int
119+
legs: JSONObject
120+
}
121+
`;
122+
123+
const bridgeText = bridge`
124+
version 1.5
125+
bridge Query.trip {
126+
with input as i
127+
with api as a
128+
with output as o
129+
130+
a.id <- i.id
131+
132+
o.id <- a.id
133+
o.legs {
134+
.duration <- a.duration
135+
.distance <- a.distance
136+
}
137+
}
138+
`;
139+
140+
const instructions = parseBridge(bridgeText);
141+
const gateway = createGateway(typeDefs, instructions, {
142+
tools: {
143+
api: async (p: any) => ({
144+
id: p.id,
145+
duration: "2h",
146+
distance: 150,
147+
}),
148+
},
149+
});
150+
const executor = buildHTTPExecutor({ fetch: gateway.fetch as any });
151+
const result: any = await executor({
152+
document: parse(`{ trip(id: 42) { id legs } }`),
153+
});
154+
155+
assert.deepStrictEqual(result.data.trip, {
156+
id: 42,
157+
legs: { duration: "2h", distance: 150 },
158+
});
159+
});
160+
161+
test("legs: [JSONObject] — array passthrough in array-mapped output", async () => {
162+
const typeDefs = /* GraphQL */ `
163+
scalar JSONObject
164+
type Query {
165+
search(from: String, to: String): [SearchResult]
166+
}
167+
type SearchResult {
168+
id: Int
169+
provider: String
170+
price: Int
171+
legs: [JSONObject]
172+
}
173+
`;
174+
175+
const bridgeText = bridge`
176+
version 1.5
177+
bridge Query.search {
178+
with input as i
179+
with api as a
180+
with output as o
181+
182+
a.from <- i.from
183+
a.to <- i.to
184+
185+
o <- a.items[] as item {
186+
.id <- item.id
187+
.provider <- item.provider
188+
.price <- item.price
189+
.legs <- item.legs
190+
}
191+
}
192+
`;
193+
194+
const instructions = parseBridge(bridgeText);
195+
const gateway = createGateway(typeDefs, instructions, {
196+
tools: {
197+
api: async () => ({
198+
items: [
199+
{ id: 1, provider: "X", price: 50, legs: [{ name: "L1" }] },
200+
{ id: 2, provider: "Y", price: 80, legs: [{ name: "L2" }] },
201+
],
202+
}),
203+
},
204+
});
205+
const executor = buildHTTPExecutor({ fetch: gateway.fetch as any });
206+
const result: any = await executor({
207+
document: parse(`{ search(from: "A", to: "B") { id legs } }`),
208+
});
209+
210+
assert.deepStrictEqual(result.data.search, [
211+
{ id: 1, legs: [{ name: "L1" }] },
212+
{ id: 2, legs: [{ name: "L2" }] },
213+
]);
214+
});
215+
});

packages/bridge/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616
"build": "tsc -p tsconfig.build.json",
1717
"prepack": "pnpm build",
1818
"lint:types": "tsc -p tsconfig.json",
19-
"test": "node --experimental-transform-types --test test/*.test.ts test/bugfixes/*.test.ts test/legacy/*.test.ts",
19+
"test": "node --experimental-transform-types --test test/*.test.ts test/bugfixes/*.test.ts test/legacy/*.test.ts test/utils/*.test.ts",
2020
"fuzz": "node --experimental-transform-types --test test/*.fuzz.ts",
21-
"test:coverage": "node --experimental-test-coverage --test-coverage-exclude=\"test/**\" --test-reporter=spec --test-reporter-destination=stdout --test-reporter=lcov --test-reporter-destination=lcov.info --experimental-transform-types --test test/*.test.ts test/bugfixes/*.test.ts test/legacy/*.test.ts",
21+
"test:coverage": "node --experimental-test-coverage --test-coverage-exclude=\"test/**\" --test-reporter=spec --test-reporter-destination=stdout --test-reporter=lcov --test-reporter-destination=lcov.info --experimental-transform-types --test test/*.test.ts test/bugfixes/*.test.ts test/legacy/*.test.ts test/utils/*.test.ts",
2222
"bench": "node --experimental-transform-types bench/engine.bench.ts",
2323
"bench:compiler": "node --experimental-transform-types bench/compiler.bench.ts"
2424
},

0 commit comments

Comments
 (0)