Skip to content
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
6 changes: 6 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,10 @@ pnpm turbo build

# run dev
pnpm turbo dev

# run typecheck
pnpm turbo typecheck # fallback when build fails due to network issues (nextjs package might fail due to font fetching issues)
```

Note: `typecheck` still rely on packages build artifacts, so it will fail if the build fails.
To handle this, you can build the `/packages/*`, then run typecheck. (when networking is not available)
3 changes: 2 additions & 1 deletion apps/backgrounds/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
"dev": "next dev --turbopack",
"build": "next build",
"start": "next start",
"lint": "next lint"
"lint": "next lint",
"typecheck": "tsc --noEmit"
},
"dependencies": {
"@next/third-parties": "15.3.2",
Expand Down
2 changes: 1 addition & 1 deletion apps/blog/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"serve": "docusaurus serve",
"write-translations": "docusaurus write-translations",
"write-heading-ids": "docusaurus write-heading-ids",
"typecheck": "tsc"
"typecheck": "tsc --noEmit"
},
"dependencies": {
"@docusaurus/core": "3.7.0",
Expand Down
3 changes: 2 additions & 1 deletion apps/viewer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
"dev": "next dev --turbopack",
"build": "next build",
"start": "next start",
"lint": "next lint"
"lint": "next lint",
"typecheck": "tsc --noEmit"
},
"packageManager": "pnpm@10.10.0",
"dependencies": {
Expand Down
22 changes: 7 additions & 15 deletions editor/grida-canvas-react/provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { domapi } from "../grida-canvas/backends/dom";
import cmath from "@grida/cmath";
import type { Action } from "@/grida-canvas/action";
import mixed, { PropertyCompareFn } from "@grida/mixed-properties";
import deepEqual from "deep-equal";
import equal from "fast-deep-equal";
import { toast } from "sonner";
import { is_direct_component_consumer } from "@/grida-canvas-utils/utils/supports";
import { Editor } from "@/grida-canvas/editor";
Expand Down Expand Up @@ -247,25 +247,17 @@ const compareNodeProperty: PropertyCompareFn<
// support gradient (as the id should be ignored)
const { id: __, ..._a } = (a ?? {}) as cg.AnyPaint;
const { id: _, ..._b } = (b ?? {}) as cg.AnyPaint;
return deepEqual(_a, _b);
return equal(_a, _b);
}
return deepEqual(a, b);
return equal(a, b);
};

export function useCurrentSelection() {
const instance = useCurrentEditor();
const state = useEditorState(instance, (state) => ({
selection: state.selection,
document: state.document,
}));

const selection = state.selection;

const nodes = useMemo(() => {
return selection.map((node_id) => {
return state.document.nodes[node_id];
});
}, [selection, state.document.nodes]);
const selection = useEditorState(instance, (state) => state.selection);
const nodes = useEditorState(instance, (state) =>
selection.map((id) => state.document.nodes[id])
);
Comment on lines +257 to +260
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential stale closure issue with refactored state management.

The refactoring introduces a potential bug where the nodes computation references the selection variable from a separate useEditorState call. This could lead to stale closure issues where the selection value used in the second useEditorState might not be the most current one.

Consider refactoring to use a single useEditorState call:

-  const selection = useEditorState(instance, (state) => state.selection);
-  const nodes = useEditorState(instance, (state) =>
-    selection.map((id) => state.document.nodes[id])
-  );
+  const { selection, nodes } = useEditorState(instance, (state) => ({
+    selection: state.selection,
+    nodes: state.selection.map((id) => state.document.nodes[id])
+  }));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const selection = useEditorState(instance, (state) => state.selection);
const nodes = useEditorState(instance, (state) =>
selection.map((id) => state.document.nodes[id])
);
const { selection, nodes } = useEditorState(instance, (state) => ({
selection: state.selection,
nodes: state.selection.map((id) => state.document.nodes[id])
}));
🤖 Prompt for AI Agents
In editor/grida-canvas-react/provider.tsx around lines 257 to 260, the code uses
two separate useEditorState calls where the second depends on the first's
selection variable, risking stale closure issues. Refactor by combining these
into a single useEditorState call that retrieves both selection and nodes
together, ensuring nodes are computed from the current selection state within
the same callback to avoid referencing outdated values.


const mixedProperties = useMemo(
() =>
Expand Down
13 changes: 12 additions & 1 deletion editor/grida-canvas-react/viewport/ui/path-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ import { cn } from "@/components/lib/utils";
import { svg } from "@/grida-canvas-utils/svg";
import assert from "assert";

function transformDelta(v: cmath.Vector2, t: cmath.Transform): cmath.Vector2 {
return cmath.vector2.transform(v, [
[t[0][0], t[0][1], 0],
[t[1][0], t[1][1], 0],
]);
}

export function SurfacePathEditor({ node_id: _node_id }: { node_id: string }) {
const { debug } = useEditorFlagsState();
const { tool } = useToolState();
Expand Down Expand Up @@ -84,7 +91,7 @@ export function SurfacePathEditor({ node_id: _node_id }: { node_id: string }) {
transform
)}
b={cmath.vector2.transform(path_cursor_position, transform)}
ta={next_ta ? next_ta : undefined}
ta={next_ta ? transformDelta(next_ta, transform) : undefined}
/>
</>
)}
Expand All @@ -93,6 +100,8 @@ export function SurfacePathEditor({ node_id: _node_id }: { node_id: string }) {
const b = vertices[s.b].p;
const ta = s.ta;
const tb = s.tb;
const ta_scaled = transformDelta(ta, transform);
const tb_scaled = transformDelta(tb, transform);
const is_neighbouring = a_point === s.a || a_point === s.b;
if (!is_neighbouring) return null;

Expand All @@ -115,6 +124,7 @@ export function SurfacePathEditor({ node_id: _node_id }: { node_id: string }) {
cmath.vector2.add(a, offset, ta),
transform
)}
ta={ta_scaled}
/>
)}
{!cmath.vector2.isZero(tb) && (
Expand All @@ -129,6 +139,7 @@ export function SurfacePathEditor({ node_id: _node_id }: { node_id: string }) {
cmath.vector2.add(b, offset, tb),
transform
)}
tb={tb_scaled}
/>
)}
{/* preview the next ta - cannot be edited */}
Expand Down
4 changes: 4 additions & 0 deletions editor/grida-canvas/editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ export class Editor
});
}

public __get_node_siblings(node_id: string): string[] {
return editor.dq.getSiblings(this.mstate.document_ctx, node_id);
}
Comment on lines +141 to +143
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve method naming and document the inherited limitation.

The new method provides a useful interface to sibling retrieval, but there are some concerns:

  1. Naming inconsistency: The __ prefix typically indicates private/internal methods, but this is marked public
  2. Inherited limitation: This method inherits the scene scoping issue from editor.dq.getSiblings

Consider these improvements:

-  public __get_node_siblings(node_id: string): string[] {
-    return editor.dq.getSiblings(this.mstate.document_ctx, node_id);
-  }
+  /**
+   * Get sibling node IDs for the given node.
+   * 
+   * @warning Root node siblings are not scoped by scene - may include 
+   * siblings from other scenes. See issue #<issue-number>
+   */
+  public getNodeSiblings(node_id: string): string[] {
+    return editor.dq.getSiblings(this.mstate.document_ctx, node_id);
+  }

This provides:

  • Consistent naming without internal prefixes for public methods
  • Clear documentation about the limitation
  • Better API discoverability
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public __get_node_siblings(node_id: string): string[] {
return editor.dq.getSiblings(this.mstate.document_ctx, node_id);
}
/**
* Get sibling node IDs for the given node.
*
* @warning Root node siblings are not scoped by scene - may include
* siblings from other scenes. See issue #<issue-number>
*/
public getNodeSiblings(node_id: string): string[] {
return editor.dq.getSiblings(this.mstate.document_ctx, node_id);
}
🤖 Prompt for AI Agents
In editor/grida-canvas/editor.ts around lines 141 to 143, rename the method from
__get_node_siblings to getNodeSiblings to remove the misleading private/internal
prefix since it is public. Add a comment above the method clearly documenting
the inherited scene scoping limitation from editor.dq.getSiblings so users are
aware of this constraint. This will improve naming consistency, clarify
limitations, and enhance API discoverability.


public dispatch(action: Action, force: boolean = false) {
if (this._locked && !force) return;
this.mstate = reducer(this.mstate, action);
Expand Down
1 change: 1 addition & 0 deletions editor/grida-canvas/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1418,6 +1418,7 @@ export namespace editor.dq {
const parent_id = getParentId(context, node_id);

if (!parent_id) {
// FIXME: this is not scoped by the scene - may result unexpected behavior.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Document the known issue with a TODO or track it separately.

The comment correctly identifies a known issue where sibling retrieval for root nodes is not scoped by scene. Consider:

  1. Adding a TODO: or FIXME: prefix to make it more visible
  2. Creating a GitHub issue to track this limitation
  3. Documenting the expected behavior vs current behavior more explicitly

Consider updating the comment to be more actionable:

-      // FIXME: this is not scoped by the scene - may result unexpected behavior.
+      // TODO: #<issue-number> - Root node siblings should be scoped by scene to prevent unexpected behavior across scenes
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// FIXME: this is not scoped by the scene - may result unexpected behavior.
// TODO: #<issue-number> - Root node siblings should be scoped by scene to prevent unexpected behavior across scenes
🤖 Prompt for AI Agents
In editor/grida-canvas/index.ts at line 1421, update the existing comment to
start with a clear TODO or FIXME prefix to highlight the known issue that
sibling retrieval for root nodes is not scoped by scene. Expand the comment to
explicitly describe the expected behavior versus the current behavior and
mention that this limitation may cause unexpected results. Additionally, create
a GitHub issue to track this problem separately for future resolution.

// If the node has no parent, it is at the root level, and all nodes without parents are its "siblings."
return Object.keys(context.__ctx_nid_to_parent_id).filter(
(id) => context.__ctx_nid_to_parent_id[id] === null
Expand Down
2 changes: 1 addition & 1 deletion editor/grida-canvas/reducers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
} from "./methods";
import eventTargetReducer from "./event-target.reducer";
import documentReducer from "./document.reducer";
import equal from "deep-equal";
import equal from "fast-deep-equal";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

LGTM! Performance improvement with dependency standardization.

The switch from deep-equal to fast-deep-equal is a good change that provides better performance while maintaining the same functionality. This aligns with similar changes across the codebase for consistency.

Since this affects the history system's change detection, please verify that the equality behavior remains consistent:


🏁 Script executed:

#!/bin/bash
# Description: Verify that fast-deep-equal is used consistently across the codebase and check for any remaining deep-equal imports

# Search for any remaining deep-equal imports
rg "from ['\"]deep-equal['\"]" --type=ts --type=js

# Search for fast-deep-equal usage to confirm consistency
rg "from ['\"]fast-deep-equal['\"]" --type=ts --type=js -A 2

Length of output: 1475


Replace all remaining deep-equal imports with fast-deep-equal

We’ve standardized on fast-deep-equal, but there are still several modules importing deep-equal. Please update these imports to avoid duplicate dependencies and ensure consistent behavior:

• editor/scaffolds/editor/feed.tsx
• editor/scaffolds/editor/reducers/database.reducer.ts
• editor/scaffolds/editor/sync/agent-theme.sync.tsx
• editor/scaffolds/editor/sync/agent-startpage.sync.tsx
• editor/grida-forms-hosted/e/interface.tsx

Example replacement:

- import equal from "deep-equal";
+ import equal from "fast-deep-equal";

After making these changes, rerun a global search to confirm no deep-equal imports remain.

Committable suggestion skipped: line range outside the PR's diff.

import grida from "@grida/schema";
import { editor } from "@/grida-canvas";
import nid from "./tools/id";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export function ImportFromGridaFileJsonDialog({
</DialogHeader>
<div className="space-y-4">
<Label>Select a .grida file</Label>
<Card className="flex items-center justify-center">
<Card className="flex items-center justify-center p-0">
<Button
onClick={openFilePicker}
disabled={loading}
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
},
"scripts": {
"test": "turbo run test",
"typecheck": "turbo run typecheck",
"publish-packages": "turbo run build lint test && changeset version && changeset publish"
},
"packageManager": "pnpm@10.10.0",
Expand Down
11 changes: 7 additions & 4 deletions packages/grida-canvas-schema/grida.ts
Original file line number Diff line number Diff line change
Expand Up @@ -542,13 +542,16 @@ export namespace grida.program.document {
__ctx_nid_to_children_ids: {},
};

// First, default every node’s parent to null
for (const node_id of ctx.__ctx_nids) {
ctx.__ctx_nid_to_parent_id[node_id] = null;
ctx.__ctx_nid_to_children_ids[node_id] = [];
}

// Then walk through and hook up actual parent/children relationships
for (const node_id in nodes) {
const node = nodes[node_id];

// Ensure the parent has an array in __ctx_nid_to_children_ids
ctx.__ctx_nid_to_children_ids[node_id] =
ctx.__ctx_nid_to_children_ids[node_id] ?? [];

// If the node has children, map each child to its parent and add to the parent’s child array
if (Array.isArray((node as nodes.UnknwonNode).children)) {
for (const child_id of (node as nodes.i.IChildrenReference)
Expand Down
4 changes: 2 additions & 2 deletions packages/grida-mixed-properties/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import deepEqual from "deep-equal";
import equal from "fast-deep-equal";

type MixedProperties<T, S> = {
[K in keyof T]: MixedProperty<T[K], S>;
Expand Down Expand Up @@ -89,7 +89,7 @@ export default function mixed<T extends Record<string, any>, S>(
idKey,
ignoredKey: ignoredKeys = [],
mixed: mixedIndicator,
compare = (_key: any, a: any, b: any) => deepEqual(a, b),
compare = (_key: any, a: any, b: any) => equal(a, b),
}: MixedOptions<T, S>
): MixedProperties<T, S> {
if (!objects.length) return {} as MixedProperties<T, S>;
Expand Down
6 changes: 2 additions & 4 deletions packages/grida-mixed-properties/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,10 @@
"dist"
],
"dependencies": {
"deep-equal": "^2.2.3"
"fast-deep-equal": "^3.1.3"
},
"jest": {
"preset": "ts-jest"
},
"devDependencies": {
"@types/deep-equal": "^1.0.4"
}
"devDependencies": {}
}
1 change: 1 addition & 0 deletions packages/grida-mixed-properties/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"compilerOptions": {
"moduleResolution": "bundler",
"esModuleInterop": true,
"noImplicitAny": true,
"strict": true
Expand Down
10 changes: 3 additions & 7 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions turbo.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
"dependsOn": ["^build"],
"outputs": [".next/**", "!.next/cache/**", "build/**", "dist/**"]
},
"check-types": {
"dependsOn": ["^check-types"]
"typecheck": {
"dependsOn": ["^typecheck"]
},
"dev": {
"persistent": true,
Expand Down