Skip to content

Commit

Permalink
Resolve dependencies from nested subgraphs (#147)
Browse files Browse the repository at this point in the history
This PR introduces a small fix to the SubgraphResolver - subgraphs are resolved recursively.
Until now we only resolved one level of subgraphs, meaning not all graphs were taken into account when resolving dependencies.
  • Loading branch information
guyca authored Jun 26, 2024
1 parent 7f0056e commit aad9207
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 12 deletions.
6 changes: 4 additions & 2 deletions packages/eslint-plugin-obsidian/rules/ast/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { TSESTree } from '@typescript-eslint/types';
import type { ArrayExpressionElement } from '../unresolvedProviderDependencies/types';
import { assertDefined } from '../utils/assertions';

export function isClassLike(node: TSESTree.Node): node is TSESTree.ClassDeclaration {
switch (node.type) {
Expand Down Expand Up @@ -35,12 +36,13 @@ export function getClassDeclaration(node: TSESTree.Node): TSESTree.ClassDeclarat
}
}

export function requireProgram(node: TSESTree.Node): TSESTree.Program {
export function requireProgram(node: TSESTree.Node | undefined): TSESTree.Program {
assertDefined(node);
switch (node.type) {
case 'Program':
return node;
default:
return requireProgram(node.parent!);
return requireProgram(node.parent);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ export class DependencyResolver {
private getDependenciesFromSubgraphs(clazz: ClassWithImports): string[] {
return this.subgraphResolver
.resolve(clazz)
.map(this.getGraphDependencies)
.flat();
.flatMap(this.getGraphDependencies);
}

private getGraphDependencies({ clazz }: ClassWithImports) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import type { FileReader } from '../framework/fileReader';
export class SubgraphResolver {
constructor(private fileReader: FileReader) { }

public resolve(clazz: ClassWithImports) {
const importedGraphs = this.getImportedGraphs(clazz);
const localGraphs = this.getLocalGraphs(clazz);
return [...importedGraphs, ...localGraphs];
public resolve(clazz: ClassWithImports): ClassWithImports[] {
return [
...this.getImportedGraphs(clazz),
...this.getLocalGraphs(clazz),
].flatMap((g) => [g, ...this.resolve(g)]);
}

private getImportedGraphs({ clazz, imports }: ClassWithImports) {
Expand All @@ -23,7 +24,7 @@ export class SubgraphResolver {

private getImportedSubgraphClasses(subgraphs: Property | undefined, imports: Import[]) {
if (!subgraphs) return [];
const subgraphNames = this.getSubgraphNames(subgraphs);
const subgraphNames = this.getSubgraphNamesFromDecoratorProperty(subgraphs);
const classes: ClassWithImports[] = [];
imports.forEach(($import) => {
// TODO: convert this to a map with an inner for each?
Expand All @@ -48,16 +49,20 @@ export class SubgraphResolver {

private getLocalSubgraphClasses(subgraphs: Property | undefined, { clazz, imports }: ClassWithImports) {
if (!subgraphs) return [];
const subgraphNames = this.getSubgraphNames(subgraphs);
const localGraphNames = this.getLocalGraphNames(subgraphNames, imports);
const allSubgraphs = this.getSubgraphNamesFromDecoratorProperty(subgraphs);
const localGraphNames = this.getLocalGraphNames(allSubgraphs, imports);
return this.createLocalGraphClasses(clazz, localGraphNames, imports);
}

private createLocalGraphClasses(clazz: Clazz, localGraphNames: string[], imports: Import[]) {
if (localGraphNames.length === 0) return [];
const parent = new File(requireProgram(clazz.node));
return localGraphNames.map((localGraphName) => {
return new ClassWithImports(parent.requireGraph(localGraphName), imports);
});
}

private getSubgraphNames(subgraphs: Property) {
private getSubgraphNamesFromDecoratorProperty(subgraphs: Property) {
return mapArrayExpression(
subgraphs.getValue<TSESTree.ArrayExpression>(),
(el) => (el as TSESTree.Identifier).name,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { Graph, ObjectGraph, Provides } from 'react-obsidian';
import Subgraph from './subgraph';

@Graph({subgraphs: [Subgraph]})
export default class GraphWithSubgraph extends ObjectGraph {
@Provides()
someString(instanceId: string, foo: string): string {
return `foo${instanceId}${foo}`;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,9 @@ export default class Subgraph extends ObjectGraph {
instanceId(): string {
return uniqueId('graph');
}

@Provides()
foo(): string {
return 'foo';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,22 @@ export default class SimpleGraphWithSubgraph extends ObjectGraph {
}
}`;

export const validGraphWithNestedSubgraphs = `
import {
Graph,
ObjectGraph,
Provides,
} from 'src';
import GraphWithSubgraph from './graphWithSubgraph';
@Graph({ subgraphs: [GraphWithSubgraph] })
export default class GraphWithNestedSubgraphs extends ObjectGraph {
@Provides()
bar(foo: string): string {
return foo + 'bar';
}
}`;

export const validFileWithTwoGraphs = `
import {
Graph,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
validFileWithTwoGraphs,
validGraph,
validGraphWithNamedExportSubgraph,
validGraphWithNestedSubgraphs,
validGraphWithRegularMethod,
validGraphWithSubgraph,
validLifecycleBoundGraphWithSubgraph,
Expand All @@ -24,6 +25,7 @@ ruleTester.run(
validFileWithTwoGraphs,
validGraphWithNamedExportSubgraph,
validGraphWithRegularMethod,
validGraphWithNestedSubgraphs,
],
invalid: [
{
Expand Down

0 comments on commit aad9207

Please sign in to comment.