Skip to content
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

Reconnect visualizations when switching their types #11494

Merged
merged 8 commits into from
Nov 13, 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
2 changes: 1 addition & 1 deletion app/gui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@
"sql-formatter": "^13.0.0",
"tar": "^6.2.1",
"tsx": "^4.7.1",
"vite-plugin-vue-devtools": "7.3.7",
"vite-plugin-vue-devtools": "7.6.3",
"vite-plugin-wasm": "^3.3.0",
"vue-react-wrapper": "^0.3.1",
"vue-tsc": "^2.0.24",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,10 @@ watchEffect(() => {
emit('update:rect', nodeOuterRect.value)
}
})

const dataSource = computed(
() => ({ type: 'node', nodeId: props.node.rootExpr.externalId }) as const,
)
</script>

<template>
Expand Down Expand Up @@ -489,7 +493,7 @@ watchEffect(() => {
:nodePosition="nodePosition"
:isCircularMenuVisible="menuVisible"
:currentType="props.node.vis?.identifier"
:dataSource="{ type: 'node', nodeId: props.node.rootExpr.externalId }"
:dataSource="dataSource"
:typename="expressionInfo?.typename"
:width="visualizationWidth"
:height="visualizationHeight"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,15 @@ watch(
() => isFullscreen,
(f) => f && nextTick(() => panelElement.value?.focus()),
)

const visParams = computed(() => {
return {
visualization: effectiveVisualization.value,
data: effectiveVisualizationData.value,
size: contentElementSize.value,
nodeType: props.typename,
}
})
</script>

<script lang="ts">
Expand Down Expand Up @@ -239,10 +248,7 @@ customElements.define(ensoVisualizationHost, defineCustomElement(VisualizationHo
>
<component
:is="ensoVisualizationHost"
:visualization="effectiveVisualization"
:data="effectiveVisualizationData"
:size="contentElementSize"
:nodeType="typename"
:params="visParams"
@updatePreprocessor="
updatePreprocessor($event.detail[0], $event.detail[1], ...$event.detail.slice(2))
"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ export const inputType = 'Any'
</script>

<script setup lang="ts">
import { watchEffect } from 'vue'

const props = defineProps<{ data: { name: string; error: Error } }>()

watchEffect(() => console.error(props.data.error))
</script>

<template>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@ import { provideVisualizationConfig } from '@/providers/visualizationConfig'
import type { Vec2 } from '@/util/data/vec2'
import type { ToValue } from '@/util/reactivity'

const { visualization, data, size, nodeType } = defineProps<{
visualization?: string | object
data?: any
size: Vec2
nodeType?: string | undefined
overflow?: boolean
toolbarOverflow?: boolean
// A single prop `params` is important to mitigate a bug in Vue that causes
// inconsistent state when multiple props are present on the custom elements component.
// TODO[ib]: Add a link to the issue.
const props = defineProps<{
params: {
visualization?: string | object
data?: any
size: Vec2
nodeType?: string | undefined
overflow?: boolean
toolbarOverflow?: boolean
}
}>()
Comment on lines +12 to 21
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To simplify usages, we could destructure props (https://vuejs.org/guide/components/props#reactive-props-destructure) - I'm not sure if it works for a second level, but event const { params } = defineProps<... should help.


const emit = defineEmits<{
Expand All @@ -32,10 +37,10 @@ const emit = defineEmits<{

provideVisualizationConfig({
get size() {
return size
return props.params.size
},
get nodeType() {
return nodeType
return props.params.nodeType
},
setPreprocessor: (
visualizationModule: string,
Expand All @@ -52,7 +57,11 @@ provideVisualizationConfig({
<template>
<Suspense>
<template #fallback><LoadingVisualization /></template>
<component :is="visualization" v-if="visualization && data" :data="data" />
<component
:is="props.params.visualization"
v-if="props.params.visualization && props.params.data"
:data="props.params.data"
/>
<LoadingVisualization v-else />
</Suspense>
</template>
Expand Down
23 changes: 17 additions & 6 deletions app/gui/src/project-view/stores/project/executionContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import * as random from 'lib0/random'
import { reactive } from 'vue'
import type { LanguageServer } from 'ydoc-shared/languageServer'
import {
methodPointerEquals,
stackItemsEqual,
type ContextId,
type Diagnostic,
Expand Down Expand Up @@ -45,16 +46,26 @@ function visualizationConfigEqual(
b: NodeVisualizationConfiguration,
): boolean {
return (
a === b ||
visualizationConfigPreprocessorEqual(a, b) &&
(a.positionalArgumentsExpressions === b.positionalArgumentsExpressions ||
(Array.isArray(a.positionalArgumentsExpressions) &&
Array.isArray(b.positionalArgumentsExpressions) &&
array.equalFlat(a.positionalArgumentsExpressions, b.positionalArgumentsExpressions)))
)
}

/** Same as {@link visualizationConfigEqual}, but ignores differences in {@link NodeVisualizationConfiguration.positionalArgumentsExpressions}. */
export function visualizationConfigPreprocessorEqual(
a: NodeVisualizationConfiguration,
b: NodeVisualizationConfiguration,
): boolean {
return (
a == b ||
(a.visualizationModule === b.visualizationModule &&
(a.positionalArgumentsExpressions === b.positionalArgumentsExpressions ||
(Array.isArray(a.positionalArgumentsExpressions) &&
Array.isArray(b.positionalArgumentsExpressions) &&
array.equalFlat(a.positionalArgumentsExpressions, b.positionalArgumentsExpressions))) &&
(a.expression === b.expression ||
(typeof a.expression === 'object' &&
typeof b.expression === 'object' &&
object.equalFlat(a.expression, b.expression))))
methodPointerEquals(a.expression, b.expression))))
)
}

Expand Down
17 changes: 13 additions & 4 deletions app/gui/src/project-view/stores/project/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Awareness } from '@/stores/awareness'
import { ComputedValueRegistry } from '@/stores/project/computedValueRegistry'
import {
ExecutionContext,
visualizationConfigPreprocessorEqual,
type NodeVisualizationConfiguration,
} from '@/stores/project/executionContext'
import { VisualizationDataRegistry } from '@/stores/project/visualizationDataRegistry'
Expand Down Expand Up @@ -237,11 +238,17 @@ export const { provideFn: provideProjectStore, injectFn: useProjectStore } = cre
})

function useVisualizationData(configuration: WatchSource<Opt<NodeVisualizationConfiguration>>) {
const id = random.uuidv4() as Uuid
const newId = () => random.uuidv4() as Uuid
const visId = ref(newId())
// Regenerate the visualization ID when the preprocessor changes.
watch(configuration, (a, b) => {
if (a != null && b != null && !visualizationConfigPreprocessorEqual(a, b))
visId.value = newId()
})

watch(
configuration,
(config, _, onCleanup) => {
[configuration, visId],
([config, id], _, onCleanup) => {
executionContext.setVisualization(id, config)
onCleanup(() => executionContext.setVisualization(id, null))
},
Expand All @@ -250,7 +257,9 @@ export const { provideFn: provideProjectStore, injectFn: useProjectStore } = cre
{ immediate: true, flush: 'post' },
)

return computed(() => parseVisualizationData(visualizationDataRegistry.getRawData(id)))
return computed(() =>
parseVisualizationData(visualizationDataRegistry.getRawData(visId.value)),
)
}

const dataflowErrors = new ReactiveMapping(computedValueRegistry.db, (id, info) => {
Expand Down
25 changes: 25 additions & 0 deletions app/gui/src/project-view/util/reactivity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import { debouncedWatch } from '@vueuse/core'
import { nop } from 'lib0/function'
import type {
ComputedRef,
// (it is used in docs)
// eslint-disable-next-line @typescript-eslint/no-unused-vars
DebuggerOptions,
DeepReadonly,
MaybeRefOrGetter,
ReactiveEffectOptions,
Expand Down Expand Up @@ -349,3 +352,25 @@ export function useSelectRef<T>(
},
})
}

/**
* Log any access to `anyRef`’s `value` property.
*
* {@link computed} and {@link watch} allow you to set custom
* {@link DebuggerOptions.onTrack} and {@link DebuggerOptions.onTrigger} callbacks,
* but they only allow tracking dependencies and recomputations, not access to the value itself.
*
* This function solves this issue by wrapping the `value` property of a ref with a custom getter.
* You can also set a debugger breakpoint in the getter to precisely track the callstack of each access.
*/
export function debugAccess(anyRef: Ref<any>, name: string) {
const originalGetter = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(anyRef), 'value')

Object.defineProperty(anyRef, 'value', {
...originalGetter,
get: () => {
console.log(`Accessing ${name}`)
return originalGetter?.get?.call(anyRef)
},
})
}
Loading
Loading