Skip to content

Fix nested object resolution #146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/quick-olives-rush.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@dmno/configraph": patch
---

fixed nested object resolution
15 changes: 14 additions & 1 deletion packages/configraph/src/config-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,16 @@ export class ConfigraphNode<NodeMetadata = any> {
}

children: Record<string, typeof this> = {};
get flatChildren(): Array<typeof this> {
if (_.isEmpty(this.children)) return [];
return _.flatMap(
_.values(this.children),
(c) => [
c,
...c.flatChildren,
],
);
}

get parentNode(): ConfigraphNode | undefined {
if (this.parent instanceof ConfigraphNode) {
Expand Down Expand Up @@ -282,13 +292,16 @@ export class ConfigraphNode<NodeMetadata = any> {
const itemResolverCtx = new ResolverContext(this.valueResolver || this);
resolverCtxAls.enterWith(itemResolverCtx);


if (this.valueResolver) {
if (!this.valueResolver.isFullyResolved) {
this.debug('running node resolver');
await this.valueResolver.resolve(itemResolverCtx);
this.isResolved = true;
if (this.resolutionError) return;
}
} else {
this.debug('no resolver - marking as resolved');
this.isResolved = true;
}

Expand Down Expand Up @@ -368,7 +381,7 @@ export class ConfigraphNode<NodeMetadata = any> {
this.isFullyResolved = true;

debug(
`${this.parentEntity?.id}/${this.path} = `,
`${this.parentEntity?.id}/${this.path} =`,
JSON.stringify(this.resolvedRawValue),
JSON.stringify(this.resolvedValue),
this.isValid ? '✅' : `❌ ${this.validationErrors?.[0]?.message}`,
Expand Down
3 changes: 1 addition & 2 deletions packages/configraph/src/entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,7 @@ export class ConfigraphEntity<

get flatConfigNodes() {
return _.flatMapDeep(this.configNodes, (node) => {
if (node.children) return [node, ..._.values(node.children)];
return node;
return [node, node.flatChildren];
});
}

Expand Down
25 changes: 20 additions & 5 deletions packages/configraph/src/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,10 @@ export class Configraph<

this.initEntitiesDag();

const postProcessFns: Array<() => void> = [];
const postProcessFns: Array<{
node: ConfigraphNode,
fns: Array<() => void>,
}> = [];
for (const entity of this.sortedEntities) {
const ancestorIds = entity.ancestorIds;

Expand Down Expand Up @@ -315,7 +318,7 @@ export class Configraph<
// calls process on each item's resolver, and collects "post-processing" functions to call if necessary
try {
const nodePostProcessFns = node.valueResolver?.process(node);
postProcessFns.push(...nodePostProcessFns || []);
postProcessFns.push({ node, fns: nodePostProcessFns || [] });
// TODO: handle errors - attach as schema errors to resolver / node?
} catch (err) {
if (err instanceof SchemaError) {
Expand All @@ -331,8 +334,18 @@ export class Configraph<

// after the entire graph of config nodes have been processed, we'll call post-processing functions
// this is needed for `collect()` where we need child entities and their nodes to be initialized
postProcessFns.forEach((postProcessFn) => {
postProcessFn();
postProcessFns.forEach(({ node, fns }) => {
fns.forEach((fn) => {
try {
fn();
} catch (err) {
if (err instanceof SchemaError) {
node.schemaErrors.push(err);
} else {
node.schemaErrors.push(new SchemaError(err as SchemaError));
}
}
});
});

// add declared dependencies to the node graph
Expand Down Expand Up @@ -361,10 +374,12 @@ export class Configraph<
const node = this.nodesByFullPath[nodeId];
// currently this resolve fn will trigger resolve on nested items
const nodeWasResolved = node.isResolved;
const nodeWasFullyResolved = node.isFullyResolved;
await node.resolve();
// for objects, the node first gets "resolved" but not "fully resolved" (where child values rolled back up)
// but this is still considered progress so we track it
// so we make progress (and therefore should continue) if a node transitions to resolved or fully resolved
if (!nodeWasResolved && node.isResolved) resolvedCount++;
else if (!nodeWasFullyResolved && node.isFullyResolved) resolvedCount++;
if (!node.isFullyResolved) {
nextBatchNodeIds.push(nodeId);
}
Expand Down
22 changes: 22 additions & 0 deletions packages/configraph/test/object-node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,4 +215,26 @@ describe('object config nodes', async () => {
});
});
});

describe('multiple nested objects', () => {
test('nested objects must completely resolve', async () => {
const g = new Configraph();
const e = g.createEntity({
configSchema: {
rootObj: {
extends: ConfigraphBaseTypes.object({
r1: {},
childObj: {
extends: ConfigraphBaseTypes.object({
c1: { value: 'c1' },
}),
},
}),
},
},
});
await g.resolveConfig();
expect(e.configNodes.rootObj.resolvedValue).toEqual({ childObj: { c1: 'c1' } });
});
});
});
Loading