From 6bfdda33a9e8d0f413a6dec7b41aeb3b8c9f75be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Grabarz?= Date: Fri, 6 Sep 2024 20:45:20 +0200 Subject: [PATCH] Do not crash on empty main function body. (#10990) Fixes #10976 https://github.com/user-attachments/assets/00b2279d-2acf-468b-8c3c-aa6885cba23d Addressed issue of empty body block being incorrectly "repaired" into an empty group, geneating invalid `()` syntax. Appending nodes to an empty function now actually replaces its body block, instead of creating a temporary orphan body block node. Note that empty main function is still considered an error on the engine side, but it doesn't impact the IDE in negative way. Things work again as soon as a node is inserted. Also fixed a few issues causing hot-reloading to break. Now edited AST code properly hot-reloads all affected modules without breaking the app. --- .../src/components/GraphEditor/GraphNode.vue | 2 +- app/gui2/src/composables/navigator.ts | 8 +-- app/gui2/src/providers/index.ts | 2 +- app/gui2/src/stores/graph/graphDatabase.ts | 4 +- app/gui2/src/stores/graph/index.ts | 1 - app/gui2/src/util/database/reactiveDb.ts | 50 ++++++++++++------- app/gui2/src/util/reactivity.ts | 3 +- app/ydoc-shared/src/ast/parse.ts | 2 + app/ydoc-shared/src/ast/tree.ts | 1 + 9 files changed, 46 insertions(+), 27 deletions(-) diff --git a/app/gui2/src/components/GraphEditor/GraphNode.vue b/app/gui2/src/components/GraphEditor/GraphNode.vue index 2545eec00f4..5502a71b36e 100644 --- a/app/gui2/src/components/GraphEditor/GraphNode.vue +++ b/app/gui2/src/components/GraphEditor/GraphNode.vue @@ -417,7 +417,7 @@ watchEffect(() => { @pointerleave="(nodeHovered = false), updateNodeHover(undefined)" @pointermove="updateNodeHover" > - + []) { let target = viewport.value for (const point of points.reverse()) target = target.offsetToInclude(point) ?? target - targetCenter.value = target.center() + targetCenter.value = target.center().finiteOrZero() } /** Pan immediately to center the viewport at the given point, in scene coordinates. */ function scrollTo(newCenter: Vec2) { resetTargetFollowing() - targetCenter.value = newCenter + targetCenter.value = newCenter.finiteOrZero() center.skip() } /** Set viewport center point and scale value immediately, skipping animations. */ function setCenterAndScale(newCenter: Vec2, newScale: number) { resetTargetFollowing() - targetCenter.value = newCenter + targetCenter.value = newCenter.finiteOrZero() targetScale.value = newScale scale.skip() center.skip() @@ -325,7 +325,7 @@ export function useNavigator( const scenePos0 = clientToScenePos(clientPos) const result = f() const scenePos1 = clientToScenePos(clientPos) - targetCenter.value = center.value.add(scenePos0.sub(scenePos1)) + targetCenter.value = center.value.add(scenePos0.sub(scenePos1)).finiteOrZero() center.skip() return result } diff --git a/app/gui2/src/providers/index.ts b/app/gui2/src/providers/index.ts index 6d8ff95eca9..a8570f3e434 100644 --- a/app/gui2/src/providers/index.ts +++ b/app/gui2/src/providers/index.ts @@ -31,7 +31,7 @@ const MISSING = Symbol('MISSING') * [Context API]: https://vuejs.org/guide/components/provide-inject.html#provide-inject */ export function createContextStore any>(name: string, factory: F) { - const provideKey = Symbol(name) as InjectionKey> + const provideKey = Symbol.for(`contextStore-${name}`) as InjectionKey> /** * Create the instance of a store and store it in the current component's context. All child diff --git a/app/gui2/src/stores/graph/graphDatabase.ts b/app/gui2/src/stores/graph/graphDatabase.ts index 5cfdad76244..a92b220d1de 100644 --- a/app/gui2/src/stores/graph/graphDatabase.ts +++ b/app/gui2/src/stores/graph/graphDatabase.ts @@ -44,7 +44,7 @@ export class BindingsDb { readFunctionAst( func: Ast.Function, - rawFunc: RawAst.Tree.Function, + rawFunc: RawAst.Tree.Function | undefined, moduleCode: string, getSpan: (id: AstId) => SourceRange | undefined, ) { @@ -422,7 +422,7 @@ export class GraphDb { /** Deeply scan the function to perform alias-analysis. */ updateBindings( functionAst_: Ast.Function, - rawFunction: RawAst.Tree.Function, + rawFunction: RawAst.Tree.Function | undefined, moduleCode: string, getSpan: (id: AstId) => SourceRange | undefined, ) { diff --git a/app/gui2/src/stores/graph/index.ts b/app/gui2/src/stores/graph/index.ts index 97d8af9f42c..2da83509b98 100644 --- a/app/gui2/src/stores/graph/index.ts +++ b/app/gui2/src/stores/graph/index.ts @@ -179,7 +179,6 @@ export const { injectFn: useGraphStore, provideFn: provideGraphStore } = createC const methodSpan = moduleSource.getSpan(method.id) assert(methodSpan != null) const rawFunc = toRaw.get(sourceRangeKey(methodSpan)) - assert(rawFunc != null) const getSpan = (id: AstId) => moduleSource.getSpan(id) db.updateBindings(method, rawFunc, moduleSource.text, getSpan) }) diff --git a/app/gui2/src/util/database/reactiveDb.ts b/app/gui2/src/util/database/reactiveDb.ts index 65b4cafd03b..bf04c394847 100644 --- a/app/gui2/src/util/database/reactiveDb.ts +++ b/app/gui2/src/util/database/reactiveDb.ts @@ -15,7 +15,15 @@ import { LazySyncEffectSet, NonReactiveView } from '@/util/reactivity' import * as map from 'lib0/map' import { ObservableV2 } from 'lib0/observable' import * as set from 'lib0/set' -import { computed, effectScope, reactive, toRaw, type ComputedRef, type DebuggerOptions } from 'vue' +import { + computed, + effectScope, + onScopeDispose, + reactive, + toRaw, + type ComputedRef, + type DebuggerOptions, +} from 'vue' export type OnDelete = (cleanupFn: () => void) => void @@ -218,14 +226,18 @@ export class ReactiveIndex { constructor(db: ReactiveDb, indexer: Indexer) { this.forward = reactive(map.create()) this.reverse = reactive(map.create()) - this.effects = new LazySyncEffectSet() - db.on('entryAdded', (key, value, onDelete) => { - const stopEffect = this.effects.lazyEffect((onCleanup) => { - const keyValues = indexer(key, value) - keyValues.forEach(([key, value]) => this.writeToIndex(key, value)) - onCleanup(() => keyValues.forEach(([key, value]) => this.removeFromIndex(key, value))) + const scope = effectScope() + this.effects = new LazySyncEffectSet(scope) + scope.run(() => { + const handler = db.on('entryAdded', (key, value, onDelete) => { + const stopEffect = this.effects.lazyEffect((onCleanup) => { + const keyValues = indexer(key, value) + keyValues.forEach(([key, value]) => this.writeToIndex(key, value)) + onCleanup(() => keyValues.forEach(([key, value]) => this.removeFromIndex(key, value))) + }) + onDelete(() => stopEffect()) }) - onDelete(() => stopEffect()) + onScopeDispose(() => db.off('entryAdded', handler)) }) } @@ -353,16 +365,20 @@ export class ReactiveMapping { */ constructor(db: ReactiveDb, indexer: Mapper, debugOptions?: DebuggerOptions) { this.computed = reactive(map.create()) - db.on('entryAdded', (key, value, onDelete) => { - const scope = effectScope() - const mappedValue = scope.run(() => - computed(() => scope.run(() => indexer(key, value)), debugOptions), - )! // This non-null assertion is SAFE, as the scope is initially active. - this.computed.set(key, mappedValue) - onDelete(() => { - scope.stop() - this.computed.delete(key) + const scope = effectScope() + scope.run(() => { + const handler = db.on('entryAdded', (key, value, onDelete) => { + const scope = effectScope() + const mappedValue = scope.run(() => + computed(() => scope.run(() => indexer(key, value)), debugOptions), + )! // This non-null assertion is SAFE, as the scope is initially active. + this.computed.set(key, mappedValue) + onDelete(() => { + scope.stop() + this.computed.delete(key) + }) }) + onScopeDispose(() => db.off('entryAdded', handler)) }) } diff --git a/app/gui2/src/util/reactivity.ts b/app/gui2/src/util/reactivity.ts index d8f9cb27ec2..bb90195f9a8 100644 --- a/app/gui2/src/util/reactivity.ts +++ b/app/gui2/src/util/reactivity.ts @@ -45,9 +45,10 @@ export type StopEffect = () => void */ export class LazySyncEffectSet { _dirtyRunners = new Set<() => void>() - _scope = effectScope() _boundFlush = this.flush.bind(this) + constructor(private _scope = effectScope()) {} + /** * Add an effect to the lazy set. The effect will run once immediately, and any subsequent runs * will be delayed until the next flush. Only effects that were notified about a dependency change diff --git a/app/ydoc-shared/src/ast/parse.ts b/app/ydoc-shared/src/ast/parse.ts index 6c06c370651..138e46ef40c 100644 --- a/app/ydoc-shared/src/ast/parse.ts +++ b/app/ydoc-shared/src/ast/parse.ts @@ -668,6 +668,8 @@ function checkSpans(expected: NodeSpanMap, encountered: NodeSpanMap, code: strin const lostBlock = new Array() for (const [key, ast] of lost) { const [start, end] = sourceRangeFromKey(key) + // Do not report lost empty body blocks, we don't want them to be considered for repair. + if (start === end && ast instanceof BodyBlock) continue ;(code.substring(start, end).match(/[\r\n]/) ? lostBlock : lostInline).push(ast) } return { lostInline, lostBlock } diff --git a/app/ydoc-shared/src/ast/tree.ts b/app/ydoc-shared/src/ast/tree.ts index 759939bef98..a63c39a7e08 100644 --- a/app/ydoc-shared/src/ast/tree.ts +++ b/app/ydoc-shared/src/ast/tree.ts @@ -1962,6 +1962,7 @@ export class MutableFunction extends Function implements MutableAst { if (oldBody instanceof MutableBodyBlock) return oldBody const newBody = BodyBlock.new([], this.module) if (oldBody) newBody.push(oldBody.take()) + this.setBody(newBody) return newBody } }