diff --git a/CHANGELOG.md b/CHANGELOG.md index 29ba9491da7..dde2327f793 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ 1. [18319](https://github.com/influxdata/influxdb/pull/18319): Display bucket ID in bucket list and enable 1 click copying 1. [18361](https://github.com/influxdata/influxdb/pull/18361): Tokens list is now consistent with the other resource lists - +1. [18346](https://github.com/influxdata/influxdb/pull/18346): Reduce the number of variables being hydrated when toggling variables ## v2.0.0-beta.11 [2020-05-26] diff --git a/flags.yml b/flags.yml index 33ee0e9cad3..62dabe2a36d 100644 --- a/flags.yml +++ b/flags.yml @@ -103,3 +103,9 @@ contact: Alirie Gray lifetime: temporary +- name: New Hydrate Vars Functionality + description: Enables a minimalistic variable hydration + key: hydratevars + default: false + contact: Ariel Salem / Monitoring Team + lifetime: temporary diff --git a/kit/feature/list.go b/kit/feature/list.go index d5b367e299b..5cd1b4455ff 100644 --- a/kit/feature/list.go +++ b/kit/feature/list.go @@ -184,6 +184,20 @@ func NewLabelPackage() BoolFlag { return newLabels } +var hydratevars = MakeBoolFlag( + "New Hydrate Vars Functionality", + "hydratevars", + "Ariel Salem / Monitoring Team", + false, + Temporary, + false, +) + +// NewHydrateVarsFunctionality - Enables a minimalistic variable hydration +func NewHydrateVarsFunctionality() BoolFlag { + return hydratevars +} + var all = []Flag{ appMetrics, backendExample, @@ -198,6 +212,7 @@ var all = []Flag{ pushDownGroupAggregateFirst, pushDownGroupAggregateLast, newLabels, + hydratevars, } var byKey = map[string]Flag{ @@ -214,4 +229,5 @@ var byKey = map[string]Flag{ "pushDownGroupAggregateFirst": pushDownGroupAggregateFirst, "pushDownGroupAggregateLast": pushDownGroupAggregateLast, "newLabels": newLabels, + "hydratevars": hydratevars, } diff --git a/ui/src/variables/actions/thunks.ts b/ui/src/variables/actions/thunks.ts index 2b038f79c40..790f6c82815 100644 --- a/ui/src/variables/actions/thunks.ts +++ b/ui/src/variables/actions/thunks.ts @@ -33,6 +33,7 @@ import {findDependentVariables} from 'src/variables/utils/exportVariables' import {getOrg} from 'src/organizations/selectors' import {getLabels, getStatus} from 'src/resources/selectors' import {currentContext} from 'src/shared/selectors/currentContext' +import {isFlagEnabled} from 'src/shared/utils/featureFlag' // Constants import * as copy from 'src/shared/copy/notifications' @@ -433,7 +434,10 @@ export const selectValue = (variableID: string, selected: string) => async ( await dispatch(selectValueInState(contextID, variableID, selected)) // only hydrate the changedVariable - dispatch(hydrateVariables(true)) - // dispatch(hydrateChangedVariable(variableID)) + if (isFlagEnabled('hydratevars')) { + dispatch(hydrateChangedVariable(variableID)) + } else { + dispatch(hydrateVariables(true)) + } dispatch(updateQueryVars({[variable.name]: selected})) } diff --git a/ui/src/variables/mocks/index.ts b/ui/src/variables/mocks/index.ts index 234366dec64..d4ffdc51a02 100644 --- a/ui/src/variables/mocks/index.ts +++ b/ui/src/variables/mocks/index.ts @@ -639,7 +639,7 @@ export const defaultVariable: Variable = { id: '05b73f4bffe8e000', orgID: '05b73f49a1d1b000', name: 'static', - description: '', + description: 'defaultVariable', selected: ['defbuck'], arguments: {type: 'constant', values: ['beans', 'defbuck']}, createdAt: '2020-05-19T05:54:20.927477-07:00', @@ -657,7 +657,7 @@ export const associatedVariable: Variable = { id: '05b740974228e000', orgID: '05b740945a91b000', name: 'dependent', - description: '', + description: 'associatedVariable', selected: [], arguments: { type: 'query', @@ -699,32 +699,36 @@ export const timeRangeStartVariable: Variable = { selected: [], } +export const timeRangeStopVariable: Variable = { + orgID: '', + id: 'timeRangeStop', + name: 'timeRangeStop', + arguments: { + type: 'system', + values: ['now()'], + }, + status: RemoteDataState.Done, + labels: [], + selected: [], +} + +export const windowPeriodVariable: Variable = { + orgID: '', + id: 'windowPeriod', + name: 'windowPeriod', + arguments: { + type: 'system', + values: [10000], + }, + status: RemoteDataState.Done, + labels: [], + selected: [], +} + export const defaultVariables: Variable[] = [ defaultVariable, associatedVariable, timeRangeStartVariable, - { - orgID: '', - id: 'timeRangeStop', - name: 'timeRangeStop', - arguments: { - type: 'system', - values: ['now()'], - }, - status: RemoteDataState.Done, - labels: [], - selected: [], - }, - { - orgID: '', - id: 'windowPeriod', - name: 'windowPeriod', - arguments: { - type: 'system', - values: [10000], - }, - status: RemoteDataState.Done, - labels: [], - selected: [], - }, + timeRangeStopVariable, + windowPeriodVariable, ] diff --git a/ui/src/variables/utils/hydrateVars.test.ts b/ui/src/variables/utils/hydrateVars.test.ts index 952232ec6a5..b736d31adbc 100644 --- a/ui/src/variables/utils/hydrateVars.test.ts +++ b/ui/src/variables/utils/hydrateVars.test.ts @@ -3,8 +3,9 @@ import {ValueFetcher} from 'src/variables/utils/ValueFetcher' import { hydrateVars, createVariableGraph, - findSubgraph, + findSubgraphFeature, } from 'src/variables/utils/hydrateVars' +import {isFlagEnabled} from 'src/shared/utils/featureFlag' // Mocks import { @@ -119,6 +120,23 @@ describe('hydrate vars', () => { // f [fontcolor = "green"] // g [fontcolor = "green"] // } + /* + We expect the final outcome to be the two associated variables as the parents, + Since those are the bottom-most children of the graph cycle + */ + if (isFlagEnabled('hydratevars')) { + expect(actual.length).toEqual(2) + expect( + actual.filter(v => v.id === 'e')[0].arguments.values.results + ).toEqual(['eVal']) + expect(actual.filter(v => v.id === 'e')[0].selected).toEqual(['eVal']) + expect( + actual.filter(v => v.id === 'g')[0].arguments.values.results + ).toEqual(['gVal']) + expect(actual.filter(v => v.id === 'g')[0].selected).toEqual(['gVal']) + } + // TODO(ariel): remove the if condition above when feature is good + // Also remove the following tests: expect( actual.filter(v => v.id === 'a')[0].arguments.values.results ).toBeFalsy() @@ -191,7 +209,14 @@ describe('hydrate vars', () => { // b [fontcolor = "red"] // c [fontcolor = "green"] // } - // + if (isFlagEnabled('hydratevars')) { + expect(actual.length).toEqual(1) + const [cResult] = actual + expect(cResult.arguments.values.results).toEqual(['cVal']) + expect(cResult.selected).toEqual(['cVal']) + } + // TODO(ariel): remove the if condition above when feature is good + // Also remove the following tests: expect( actual.filter(v => v.id === 'a')[0].arguments.values.results ).toEqual([]) @@ -244,6 +269,15 @@ describe('hydrate vars', () => { // Basic test for now, we would need an icky mock to assert that the // appropriate substitution is actually taking place + if (isFlagEnabled('hydratevars')) { + expect(actual.length).toEqual(1) + const [bResult] = actual + expect(bResult.arguments.values).toEqual({ + k: 'v', + }) + } + // TODO(ariel): remove the if condition above when feature is good + // Also remove the following tests: expect( actual.filter(v => v.id === 'a')[0].arguments.values.results ).toEqual(['aVal']) @@ -290,6 +324,13 @@ describe('hydrate vars', () => { fetcher, }).promise + if (isFlagEnabled('hydratevars')) { + expect(actual.length).toEqual(1) + const [bResult] = actual + expect(bResult.arguments.values).toEqual(['v1', 'v2']) + } + // TODO(ariel): remove the if condition above when feature is good + // Also remove the following tests: expect( actual.filter(v => v.id === 'a')[0].arguments.values.results ).toEqual(['aVal']) @@ -323,24 +364,74 @@ describe('hydrate vars', () => { cancel() }) -}) -xdescribe('findSubgraph', () => { - test('should return the update variable with all associated parents', async () => { - const variableGraph = await createVariableGraph(defaultVariables) - const actual = await findSubgraph(variableGraph, [defaultVariable]) - // returns the single subgraph result - expect(actual.length).toEqual(1) - const [subgraph] = actual - // expect the subgraph to return the passed in variable - expect(subgraph.variable).toEqual(defaultVariable) - // expect the parent to be returned with the returning variable - expect(subgraph.parents[0].variable).toEqual(associatedVariable) + test('should return the child node with associated parents', async () => { + /* + This example deals with the following situation where a parent with a child has been passed in. + Practically speaking, this looks like: + + associatedVariable + | + | + defaultVariable + + By passing in the defaultVariable, we expect the child to be returned with a reference to the parent: + + [ + { + variable: defaultVariable, + parent: [associatedVariable] + }, + ] + The reason for this is because we want the youngest child node (end of the LL tail) to load first, followed by its parents. + */ + + const fetcher = new FakeFetcher() + + const actual = await hydrateVars([associatedVariable], defaultVariables, { + url: '', + orgID: '', + selections: {}, + fetcher, + }).promise + if (isFlagEnabled('hydratevars')) { + expect(actual.length).toEqual(1) + expect(actual).toEqual([defaultVariable]) + } + // TODO(ariel): remove the if condition above when feature is good + // Also remove the following tests: + expect(actual.length).toEqual(2) }) +}) + +describe('findSubgraphFeature', () => { + const getParentNodeIDs = (node, acc: Set = new Set()): string[] => { + for (const parent of node.parents) { + if (!acc.has(parent)) { + acc.add(parent.variable.id) + getParentNodeIDs(parent, acc) + } + } + return [...acc] + } + test('should return the variable with no parents when no association exists', async () => { + /* + This example deals with the following situation where a node with no relationship is passed in: + By passing in the variable `a`, we expect the child to be returned with a reference to the parent. + However, since no children or parents exist, the following should be: + + [ + { + variable: a, + parent: [] + children: [], + }, + ] + */ const a = createVariable('a', 'f(x: v.b)') const variableGraph = await createVariableGraph([...defaultVariables, a]) - const actual = await findSubgraph(variableGraph, [a]) + const actual = await findSubgraphFeature(variableGraph, [a]) expect(actual.length).toEqual(1) const [subgraph] = actual // expect the subgraph to return the passed in variable @@ -349,8 +440,28 @@ xdescribe('findSubgraph', () => { expect(subgraph.parents).toEqual([]) }) test('should return the update default (timeRange) variable with associated parents', async () => { + /* + This example deals with the following situation where a parent with a child has been passed in: + + associatedVariable + | + | + timeRangeStart + + By passing in the timeRangeStart, we expect the child to be returned with a reference to the parent: + + [ + { + variable: timeRangeStart, + parent: [associatedVariable] + }, + ] + The reason for this is because we want the youngest child node (end of the LL tail) to load first, followed by its parents. + */ const variableGraph = await createVariableGraph(defaultVariables) - const actual = await findSubgraph(variableGraph, [timeRangeStartVariable]) + const actual = await findSubgraphFeature(variableGraph, [ + timeRangeStartVariable, + ]) expect(actual.length).toEqual(1) const [subgraph] = actual // expect the subgraph to return the passed in variable @@ -358,4 +469,212 @@ xdescribe('findSubgraph', () => { // expect the parent to be returned with the returning variable expect(subgraph.parents[0].variable).toEqual(associatedVariable) }) + test('should filter out inputs that have already been loaded based on a previous associated variable', async () => { + /* + This example deals with the following situation where a parent with a child has been passed in, and a unrelated variable is passed in. + Practically speaking, this looks like: + + associatedVariable a + | + | + defaultVariable + + By passing in the defaultVariable, we expect the child to be returned with a reference to the parent. + Since the variable `a` does not have any parent/child relationship, we expect it to simply be returned: + + [ + { + variable: defaultVariable, + parent: [associatedVariable] + }, + + variable: timeRangeStart, + parent: [] + }, + { + variable: timeRangeStop, + parent: [] + }, + { + variable: a, + children: [], + parent: [], + } + ] + The reason for this is because we want the youngest child node (end of the LL tail) to load first, followed by its parents. + Since defaultVariable is the youngest child node, and since `a` doesn't have a child node, + we expect those values to be returned with any references to their parents + */ + const a = createVariable('a', 'f()') + const variableGraph = await createVariableGraph([...defaultVariables, a]) + const actual = await findSubgraphFeature(variableGraph, [ + defaultVariable, + associatedVariable, + a, + ]) + // returns the four subgraph results + expect(actual.length).toEqual(4) + const [defaultVarInResult] = actual.filter( + vn => vn.variable.id === defaultVariable.id + ) + expect(defaultVarInResult.variable).toEqual(defaultVariable) + // expect the subgraph to return the passed in variable + const parentIDs = getParentNodeIDs(defaultVarInResult) + expect(parentIDs.length).toEqual(1) + const [parentID] = parentIDs + // expect the defaultVariable to have the associatedVariable as a parent + expect(parentID).toEqual(associatedVariable.id) + const [timeRangeResult] = actual.filter( + vn => vn.variable.id === timeRangeStartVariable.id + ) + expect(timeRangeResult).toEqual(timeRangeResult) + // expect the subgraph to return the passed in variable + const rents = getParentNodeIDs(timeRangeResult) + expect(rents).toEqual([]) + }) + test('should return the child node (defaultVariable) with associated parents of the input (associatedVariable) when the child node is passed in', async () => { + /* + This example deals with the following situation where a parent with a child has been passed in. + Practically speaking, this looks like: + + associatedVariable + | + | + defaultVariable + + By passing in the defaultVariable, we expect the child to be returned with a reference to the parent: + + [ + { + variable: defaultVariable, + parent: [associatedVariable] + }, + ] + The reason for this is because we want the youngest child node (end of the LL tail) to load first, followed by its parents. + */ + const variableGraph = await createVariableGraph(defaultVariables) + const actual = await findSubgraphFeature(variableGraph, [defaultVariable]) + // returns the subgraph result + expect(actual.length).toEqual(1) + const resultIDs = actual.map(v => v.variable.id) + // expect the one variables with no children to be output + expect(resultIDs).toEqual([defaultVariable.id]) + expect(actual[0].children).toEqual([]) + // expect the subgraph to return the passed in variable + const parentIDs = getParentNodeIDs(actual[0]) + expect(parentIDs.length).toEqual(1) + const [parentID] = parentIDs + // expect the defaultVariable to have the associatedVariable as a parent + expect(parentID).toEqual(associatedVariable.id) + }) + test('should return all the child nodes if the input variable has children', async () => { + /* + This example deals with the following situation where a parent with multiple children has been passed in. + Practically speaking, this looks like: + + associatedVariable + / | \ + / | \ + timeRangeStart defaultVariable timeRangeStop + + By passing in the associatedVariable, we expect the first to be returned with a reference to the parent, + while the subsequent children should not include a reference to the parent: + + [ + { + variable: defaultVariable, + parent: [associatedVariable] + }, + { + variable: timeRangeStart, + parent: [] + }, + { + variable: timeRangeStop, + parent: [] + }, + ] + The reason for this is because we want the youngest child node (end of the LL tail) to load first, followed by its parents. + */ + const variableGraph = await createVariableGraph(defaultVariables) + const actual = await findSubgraphFeature(variableGraph, [ + associatedVariable, + ]) + // returns the subgraph result + expect(actual.length).toEqual(3) + // expect the one variables with no children to be output + const [defaultVarInResult] = actual.filter( + vn => vn.variable.id === defaultVariable.id + ) + expect(defaultVarInResult.variable).toEqual(defaultVariable) + // expect the subgraph to return the passed in variable + const parents = getParentNodeIDs(defaultVarInResult) + expect(parents.length).toEqual(1) + const [parent] = parents + // expect the defaultVariable to have the associatedVariable as a parent + expect(parent).toEqual(associatedVariable.id) + const [timeRangeResult] = actual.filter( + vn => vn.variable.id === timeRangeStartVariable.id + ) + expect(timeRangeResult).toEqual(timeRangeResult) + // expect the subgraph to return the passed in variable + const rents = getParentNodeIDs(timeRangeResult) + expect(rents).toEqual([]) + }) + test('should only return the child nodes (defaultVariable, timeRangeStart, timeRangeStop) with the like parents filtered out of the second variable (timeRangeStart) when the multiple common parents are passed in', async () => { + /* + This example deals with the following situation where a parent has two children. + In this case, both the defaultVariable and timeRangeStart are children to associatedVariable. + Practically speaking, this looks like: + + associatedVariable + / | \ + / | \ + timeRangeStart defaultVariable timeRangeStop + + By passing in the associatedVariable and the timeRangeStart, the output should be: + + [ + { + variable: defaultVariable, + parent: [associatedVariable] + }, + { + variable: timeRangeStart, + parent: [], + }, + { + variable: timeRangeStop, + parent: [], + }, + ] + The reason for this is because we deduplicate parent nodes that have already been loaded. + Since the defaultVariable has already hydrated the associatedVariable, we can + rely upon that in order to resolve timeRangeStart + */ + const variableGraph = await createVariableGraph(defaultVariables) + const actual = await findSubgraphFeature(variableGraph, [ + timeRangeStartVariable, + associatedVariable, + ]) + // returns the subgraph result + expect(actual.length).toEqual(3) + const [defaultVarInResult] = actual.filter( + vn => vn.variable.id === defaultVariable.id + ) + expect(defaultVarInResult.variable).toEqual(defaultVariable) + // expect the subgraph to return the passed in variable + const parentIDs = getParentNodeIDs(defaultVarInResult) + expect(parentIDs.length).toEqual(1) + const [parentID] = parentIDs + // expect the defaultVariable to have the associatedVariable as a parent + expect(parentID).toEqual(associatedVariable.id) + const [timeRangeResult] = actual.filter( + vn => vn.variable.id === timeRangeStartVariable.id + ) + expect(timeRangeResult).toEqual(timeRangeResult) + // expect the subgraph to return the passed in variable + const rents = getParentNodeIDs(timeRangeResult) + expect(rents).toEqual([]) + }) }) diff --git a/ui/src/variables/utils/hydrateVars.ts b/ui/src/variables/utils/hydrateVars.ts index d11d1864b0a..289937155b6 100644 --- a/ui/src/variables/utils/hydrateVars.ts +++ b/ui/src/variables/utils/hydrateVars.ts @@ -2,6 +2,7 @@ import {valueFetcher, ValueFetcher} from 'src/variables/utils/ValueFetcher' import Deferred from 'src/utils/Deferred' import {asAssignment} from 'src/variables/selectors' +import {isFlagEnabled} from 'src/shared/utils/featureFlag' // Constants import {OPTION_NAME, BOUNDARY_GROUP} from 'src/variables/constants/index' @@ -73,7 +74,7 @@ export const createVariableGraph = ( return Object.values(nodesByID) } -export const isInQuery = (query: string, v: Variable) => { +export const isInQuery = (query: string, v: Variable): boolean => { const regexp = new RegExp( `${BOUNDARY_GROUP}${OPTION_NAME}.${v.name}${BOUNDARY_GROUP}` ) @@ -111,6 +112,78 @@ const collectAncestors = ( return [...acc] } +/* + Using the current node as the root, create a PostOrder DFS for the given node + Where we check each cascading parent's children in order to hydrate the children before the parents + + In the example below, we have 3 variables `a`, `b`, `c` where `a` is a parent to `b`, which a parent to `c`: + + a --> b --> c + + If `a`, `b`, or `c` are passed into this function, we should expect that the output for each should be: + + `c` with a reference to its parent `b`, which should have a reference to its parent `a` +*/ + +const postOrderDFS = ( + node: VariableNode, + acc: Set = new Set(), + cache: {[key: string]: boolean | undefined} = {} +): VariableNode[] => { + // Handle the edge case that the function is + // called without providing the root node + if (!node) return [...acc] + + for (const child of node.children) { + // by checking the cache for existing variables, we ensure that the graph stops + // when an existing node has already been encountered, invalidating a cycle if one exists + if (!cache[child.variable.id]) { + cache[child.variable.id] = true + // Recurse down the n-ary tree until we reach a leaf (node with no children) + // where this loop will not execute + postOrderDFS(child, acc, cache) + } + } + + // Add the current node only after visiting all of its children, left to right + acc.add(node) + + return [...acc] +} + +/* + Filters out any parents that have already been referenced in a previous node + + A node `a` & `c` are children of `b` if there exists a path: + + b + / \ + a c + + This function safely filters out a duplicate reference to the parent `b` + When both `a` and `c` are passed as variables to be hydrated, since `b` should + already be `hydrated` by the first variable that is passed in. +*/ +export interface DeduplicatedRoot { + node: VariableNode + subsetIDs: {[key: string]: boolean} +} + +const getDeduplicatedRootChild = ( + node: VariableNode, + subsetIDs: {[key: string]: boolean | undefined} +): DeduplicatedRoot => { + for (const n of node.parents) { + if (!subsetIDs[n.variable.id]) { + subsetIDs[n.variable.id] = true + getDeduplicatedRootChild(n, subsetIDs) + } else { + node.parents = [] + } + } + return {node, subsetIDs} +} + /* Given a variable graph, return the minimal subgraph containing only the nodes needed to hydrate the values for variables in the passed `variables` argument. @@ -121,6 +194,54 @@ const collectAncestors = ( - The node for one of the passed variables depends on this node */ +// TODO(ariel): rename this back to findSubgraph & update tests once feature flag is removed +export const findSubgraphFeature = ( + graph: VariableNode[], + variables: Variable[] +): VariableNode[] => { + const subgraph: Set = new Set() + // use an ID array to reduce the chance of reference errors + const varIDs = variables.map(v => v.id) + // create an ID reference object to identify relevant root variables to hydrate + let subgraphIDs = {} + for (const node of graph) { + const shouldKeep = varIDs.includes(node.variable.id) + if (shouldKeep) { + const postOrderNodeList = postOrderDFS(node) + postOrderNodeList.forEach(variableNode => { + const {id} = variableNode.variable + // Checking whether the subgraphIDs[id] !== true, we are ensuring that the + // node has been added to the subgraph. This prevents excessive variable rehydration + if (subgraphIDs[id] !== true) { + /* + Once a node exists within the subgraph (whether nested as a parent or as the input node) + we want to remove any further reference to that node within the subgraph. + This can be particularly challenging when a parent node has multiple child nodes that have been passed in. + For example, if variables `a` and `b` are both children to `c`, + and `a` & `b` are both variables that should be hydrated, we would need to reset a parent + reference for one of the variables so that `c` is not hydrated twice. + This can be achieved by storing a reference to all the existing nodeIDs within the subgraph to the `graphIDs` + and checking for any collisions before adding the node to the subgraph. + If a parent collision is detected, that node's parent is simply set to [] since we know that + the parent was already hydrated based on a previous input + */ + const {node: filteredNodes, subsetIDs} = getDeduplicatedRootChild( + variableNode, + subgraphIDs + ) + subgraph.add(filteredNodes) + subgraphIDs = { + [id]: true, + ...subgraphIDs, + ...subsetIDs, + } + } + }) + } + } + + return [...subgraph] +} export const findSubgraph = ( graph: VariableNode[], variables: Variable[] @@ -128,9 +249,6 @@ export const findSubgraph = ( const subgraph: Set = new Set() // use an ID array to reduce the chance of reference errors const varIDs = variables.map(v => v.id) - // TODO: uncomment this when variable hydration is resolved - // create an array of IDs to reference later - // const graphIDs = [] for (const node of graph) { const shouldKeep = varIDs.includes(node.variable.id) || @@ -140,18 +258,10 @@ export const findSubgraph = ( if (shouldKeep) { subgraph.add(node) - // graphIDs.push(node.variable.id) } } - // const removeDupAncestors = (n: VariableNode) => { - // const {id} = n.variable - // return !graphIDs.includes(id) - // } - for (const node of subgraph) { - // node.parents = node.parents.filter(removeDupAncestors) - // node.children = node.children.filter(removeDupAncestors) node.parents = node.parents.filter(node => subgraph.has(node)) node.children = node.children.filter(node => subgraph.has(node)) } @@ -290,7 +400,7 @@ const findLeaves = (graph: VariableNode[]): VariableNode[] => */ const findCyclicPath = (node: VariableNode): VariableNode[] => { try { - findCyclicPathHelper(node, []) + findCyclicPathHelper(node, [], {}) } catch (cyclicPath) { return cyclicPath } @@ -300,14 +410,18 @@ const findCyclicPath = (node: VariableNode): VariableNode[] => { const findCyclicPathHelper = ( node: VariableNode, - seen: VariableNode[] + seen: VariableNode[], + cache: {[key: string]: undefined | boolean} = {} ): void => { - if (seen.includes(node)) { + if (cache[node.variable.id]) { throw seen } for (const child of node.children) { - findCyclicPathHelper(child, [...seen, node]) + findCyclicPathHelper(child, [...seen, node], { + ...cache, + [node.variable.id]: true, + }) } } @@ -399,7 +513,12 @@ export const hydrateVars = ( allVariables: Variable[], options: HydrateVarsOptions ): EventedCancelBox => { - const graph = findSubgraph( + let findSubgraphFunction = findSubgraph + if (isFlagEnabled('hydratevars')) { + findSubgraphFunction = findSubgraphFeature + } + + const graph = findSubgraphFunction( createVariableGraph(allVariables), variables ).filter(n => n.variable.arguments.type !== 'system') @@ -411,7 +530,6 @@ export const hydrateVars = ( if (isCancelled) { return } - try { // TODO: terminate the concept of node.values at the fetcher and just use variables node.values = await hydrateVarsHelper(node, options, on)