-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
fix: hierarchy of variables is back #17609
Conversation
@@ -115,6 +116,8 @@ export const executeQueries = () => async (dispatch, getState: GetState) => { | |||
try { | |||
dispatch(setQueryResults(RemoteDataState.Loading, [], null)) | |||
|
|||
await dispatch(hydrateVariables()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this refreshes variables that haven't been fetched by another means before sending off the query, just in case. found this while deep linking w/ dependent variables and no application state.
uses built in cache mechanism to ensure we're not over fetching
@@ -175,6 +175,11 @@ export const getVariable = ( | |||
if (!vari.selected) { | |||
if (vari.arguments.type === 'map') { | |||
vari.selected = [Object.keys(vari.arguments.values)[0]] | |||
} else if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated change, just an oversight I caught while debugging
ui/src/variables/selectors/index.tsx
Outdated
@@ -252,6 +257,9 @@ export const asAssignment = (variable: Variable): VariableAssignment => { | |||
} | |||
|
|||
if (variable.arguments.type === 'query') { | |||
if (!variable.selected[0]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the ensures that the AST doesn't return variables if they don't have a value set, because that causes all sorts of issues in the LSP and on the api
@@ -20,7 +20,7 @@ export const buildVarsOption = (variables: VariableAssignment[]): File => ({ | |||
}, | |||
init: { | |||
type: 'ObjectExpression', | |||
properties: variables.map(assignmentToProperty), | |||
properties: variables.filter(v => !!v).map(assignmentToProperty), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this makes sure those variables we removed are filtered from the list before further processing
node.values = await hydrateVarsHelper(node, options) | ||
node.variable.arguments.values.results = node.values.values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here is the actual fix for this PR. there's more to be done to further enforce the variable data structure, as mentioned in the TODO
Closes #17606
variables now hold their own values, and we gotta respect that, you know?