refig - render images from .fig or cached figma rest-api response#539
Merged
softmarshmallow merged 35 commits intomainfrom Feb 17, 2026
Merged
refig - render images from .fig or cached figma rest-api response#539softmarshmallow merged 35 commits intomainfrom
refig - render images from .fig or cached figma rest-api response#539softmarshmallow merged 35 commits intomainfrom
Conversation
- Revised the description to specify that the package accepts both exported `.fig` files and Figma REST API document JSON responses. - Added a new section with an example of rendering from Figma REST API document JSON. - Updated the "Who is this NOT for?" section to clarify the package's focus on headless rendering without built-in API fetching.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
6 Skipped Deployments
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds caller-specified image RID support across runtime, WASM bindings, editor, IO and archive formats; introduces the new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/Editor
participant Canvas as Canvas API
participant WASM as WASM Runtime
participant Renderer as Renderer
participant ImageRepo as Image Repository
rect rgba(200,200,255,0.5)
Client->>Canvas: addImageWithId(imageData, rid)
Canvas->>WASM: _add_image_with_rid(data_ptr,data_len,rid_ptr,rid_len)
WASM->>Renderer: add_image_with_rid(app,data_slice,rid_str)
Renderer->>ImageRepo: validate RID prefix && store bytes under rid
Renderer->>Renderer: detect width,height,mime and build JSON result
Renderer-->>WASM: return length-prefixed JSON ptr
WASM-->>Canvas: ptr -> parsed result
Canvas-->>Client: CreateImageResourceResult | false
end
sequenceDiagram
participant Editor as Editor Instance
participant Import as Figma Import (iofigma)
participant Factory as FactoryContext
participant ImageMgr as Image Manager
participant Canvas as Canvas/WASM
rect rgba(200,255,200,0.5)
Editor->>Import: import REST JSON + optional images
Import->>Factory: create FactoryContext(resolve_image_src,generators)
Factory->>Import: convertPageToScene(...) -> { document, imageRefsUsed }
Import->>ImageMgr: fetch/collect images for imageRefsUsed
ImageMgr->>Canvas: instance.loadImages(record<ref,bytes>)
Canvas->>WASM: register images with custom RIDs
Import-->>Editor: insert converted document (refs preserved)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Added core functionality to render Figma documents from both `.fig` files and REST API JSON responses. - Introduced CLI for headless rendering, supporting multiple output formats (PNG, JPEG, WebP, PDF, SVG). - Created Node.js and browser entry points, with the Node version including file I/O helpers. - Implemented utility functions for export settings and rendering options. - Added tests for core functionalities and CLI operations to ensure reliability. - Updated README with usage examples and installation instructions.
- Introduced a new Python script `figma_archive.py` for archiving Figma files via the REST API, including downloading document data and images. - Updated `.gitignore` to include Python bytecode and cache directories, ensuring cleaner repository management.
- Added `add_image_with_rid(bytes, rid)` method to allow registration of image bytes under a caller-specified RID, supporting logical identifiers. - Updated `README.md` to document the new image registration functionality. - Introduced integration tests to verify the behavior of the new method, including validation of RID formats and retrieval of registered images. - Enhanced WASM bindings to expose the new functionality for use in web applications.
- Added a link to the `figma_archive.py` script in the README for easier access to Figma file archiving via REST API. - Updated `figma_archive.py` to improve code readability by formatting the content type extraction. - Included detailed instructions for performing end-to-end tests using the archiving script, enhancing user guidance for local project setup.
- Added a new module for loading Figma default fonts from CDN, ensuring proper font registration and fallback to reduce missing glyphs in mixed-script and CJK text. - Introduced `loadFigmaDefaultFonts` option in the renderer configuration to control automatic font loading. - Updated `@grida/canvas-wasm` dependency to version 0.90.0-canary.7. - Added tests for the new font loading functionality and updated documentation to reflect these changes.
- Introduced a new type `VectorNetwork_restapi_volatile20260217` to handle the undocumented vector network shape returned by the Figma REST API. - Updated `VectorNodeRestInput` to optionally include the `vectorNetwork` field. - Implemented `normalizeRestVectorNetworkToIR` function to normalize the volatile vector network shape to the internal representation. - Enhanced tests to validate conversion of VECTOR nodes with vectorNetwork and fallback behavior for invalid networks. - Added a minimal valid vectorNetwork fixture for testing purposes.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b0a291800
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+693
to
+696
| let exportAs = mapFormat(format); | ||
| if (scale !== 1) { | ||
| exportAs = applyScale(exportAs, scale); | ||
| } |
There was a problem hiding this comment.
render() only applies renderOptions.scale to exportAs and otherwise leaves mapFormat() at constraints: { type: "none" }, so WIDTH/HEIGHT export settings are effectively ignored. This affects --export-all and API calls that come from exportSettingToRenderOptions() for constraint.type === "WIDTH" | "HEIGHT": outputs are rendered at intrinsic node size instead of the requested size.
Useful? React with 👍 / 👎.
| for (const [key, imageData] of Object.entries(_x_images)) { | ||
| const base = key.split("/").pop() ?? key; | ||
| const hashHex = base.includes(".") ? base.split(".")[0]! : base; | ||
| const ref = base.includes(".") ? base.split(".")[0]! : base; |
There was a problem hiding this comment.
Archive unpack currently derives refs with base.split(".")[0], which truncates refs at the first dot (and after split("/").pop() also drops any path segments). With this commit introducing custom RID-based refs (for example res://images/logo.png), reopening an archive can remap the stored image to a different key and break ImagePaint.src lookups, causing missing images.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/grida-canvas-io/index.ts (1)
780-798:⚠️ Potential issue | 🟡 MinorRef extraction may truncate filenames with multiple dots.
base.split(".")[0]on a filename likemy.image.pngyieldsmyinstead ofmy.image. If image filenames can contain dots before the extension, this would produce incorrect refs. The same pattern is used inarchive.pack(Line 977), so it's at least internally consistent, but worth noting.If filenames with multiple dots are possible, consider using
base.replace(/\.[^.]+$/, "")to strip only the final extension.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-io/index.ts` around lines 780 - 798, The ref extraction currently splits the filename on "." and takes the first segment (in the loop over _x_images), which will truncate names like "my.image.png"; update the logic that computes base/ref (used where assets[ref] and imagesRepo[ref] are set) to strip only the final extension (e.g., use a replace with /\.[^.]+$/ or use lastIndexOf to remove the suffix) so "my.image.png" becomes "my.image" instead of "my"; apply the same fix to the corresponding logic in archive.pack to keep behavior consistent.
🧹 Nitpick comments (17)
apps/blog/blog/refig/post.md (1)
53-55: Vary the bullet openers to reduce repetition.The three bullets start with “No …” in sequence; consider rewording one or two for a smoother read.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blog/blog/refig/post.md` around lines 53 - 55, The three consecutive bullets starting with "**No auth / fetching**", "**No design-to-code**", and "**No editor**" feel repetitive—rephrase one or two to vary the openers while preserving meaning; for example, change "**No auth / fetching**" to "Bring your own token storage, HTTP client, and caching", or change "**No design-to-code**" to "Rendering only (pixels / SVG / PDF), not HTML/CSS/Flutter generation", or change "**No editor**" to "Read-only: render content without editing"—apply one or two of these rewordings to the bullets to smooth the rhythm.packages/grida-canvas-io/__tests__/format-roundtrip.test.ts (1)
2424-2473: Missingpaint.srcassertion in stroke ImagePaint roundtrip test.The
srcwas updated to"res://images/fedcba9876543210"on Line 2434, but the assertions block (Lines 2462–2469) never verifies thatpaint.srcsurvived the roundtrip. The fill-paint counterpart tests do assertsrc. Consider adding:if (paint && paint.type === "image") { + expect(paint.src).toBe("res://images/fedcba9876543210"); expect(paint.fit).toBe("cover");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-io/__tests__/format-roundtrip.test.ts` around lines 2424 - 2473, The stroke ImagePaint roundtrip test ("roundtrips ImagePaint stroke_paints on ContainerNode") is missing an assertion for paint.src; update the assertion block inside the roundtripTest callback to verify the image source survived the roundtrip by asserting paint.src equals "res://images/fedcba9876543210" (use the same pattern as the fill-paint counterpart and the existing checks for paint.fit, paint.blend_mode, paint.opacity, paint.active, and paint.filters).editor/grida-canvas-react-starter-kit/starterkit-import/from-figma.tsx (1)
436-445:onClose()runs in.finally()even when the now-awaitableonImportrejects.With
onImportnow potentially async and long-running, a rejection inside it will close the dialog (via.finally) before the user can act on the error toast. Consider movingonClose()into the.then()block so the dialog stays open on failure.Suggested change
.then(async (r) => { await onImport?.(r); + onClose(); }) .catch((e) => { console.error(e); throw e; - }) - .finally(() => { - onClose(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor/grida-canvas-react-starter-kit/starterkit-import/from-figma.tsx` around lines 436 - 445, The current promise chain calls onClose() in .finally(), which closes the dialog even if the async onImport(r) rejects; move the onClose() call out of .finally() and invoke it only after successful completion of onImport — i.e., inside the .then(async (r) => { await onImport?.(r); onClose(); }) block (or after awaiting onImport in a try block) so that the dialog remains open when onImport throws, while still allowing errors to propagate to the .catch handler.packages/grida-canvas-sdk-render-figma/__tests__/refig.test.ts (1)
353-373: Scale render test doesn't verify output dimensions.Other render tests (e.g., lines 276-277, 418-419) assert
result.widthandresult.height. This test passesscale: 2but only checks the PNG signature — it would be useful to assert on the resulting dimensions to verify the scale option actually has an effect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/__tests__/refig.test.ts` around lines 353 - 373, The test for scaling using FigmaRenderer.render currently only checks the PNG signature; update the "renders with scale option" test to also assert that result.width and result.height reflect the scale (i.e., width === 256 * scale and height === 256 * scale when calling renderer.render("1:1", { width: 256, height: 256, scale: 2 })); locate the test case using the FigmaRenderer instance and the render invocation to add the two expect assertions for result.width and result.height before writing the file and disposing the renderer.packages/grida-canvas-io-figma/lib.ts (2)
1420-1456: Silentcatchswallows all errors — consider logging for debuggability.The empty
catchblock at line 1453 silently discards any exception fromnormalizeRestVectorNetworkToIRor the vector network conversion. Elsewhere in this file (line 1009), the same kind of failure logs aconsole.warn. Adding a similar warning here would help diagnose unexpected failures during volatile API normalization.Suggested fix
- } catch { - // Fall through to GroupNode + fillGeometry/strokeGeometry path - } + } catch (e) { + console.warn( + `Failed to normalize volatile REST vectorNetwork (${node.name}), falling back to GroupNode:`, + e + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-io-figma/lib.ts` around lines 1420 - 1456, The empty catch in the VECTOR case swallows errors from restful.map.normalizeRestVectorNetworkToIR and the gridaVectorNetwork conversion; update that catch to log a warning (e.g., console.warn or processLogger.warn) including the caught error and contextual identifiers (such as node.id or gridaId) and a short message indicating vectorNetwork normalization failed so debugging is possible—apply this change in the catch following the normalizeRestVectorNetworkToIR call inside the VECTOR case.
1949-1996:fmthelper doesn't guard against negative or non-integer numeric enum values.
typeof t === "number" && t < 4allows negative numbers or floats to index intoFMT, which would yieldundefined. While Kiwi likely only produces small non-negative integers, a lower-bound check would be more defensive.Suggested fix
const fmt = (t: string | number | undefined) => typeof t === "string" ? (FMT_STR[t] ?? null) - : typeof t === "number" && t < 4 + : typeof t === "number" && Number.isInteger(t) && t >= 0 && t < 4 ? FMT[t] : null; const cstr = (t: string | number | undefined) => typeof t === "string" ? (CSTR_STR[t] ?? "SCALE") - : typeof t === "number" && t < 3 + : typeof t === "number" && Number.isInteger(t) && t >= 0 && t < 3 ? CSTR[t] : "SCALE";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-io-figma/lib.ts` around lines 1949 - 1996, The fmt numeric branch is too permissive and can index FMT with negative or non-integer values; update the numeric checks in kiwi_has_export_settings_trait so fmt uses Number.isInteger(t) && t >= 0 && t < FMT.length (instead of t < 4) and similarly make cstr use Number.isInteger(t) && t >= 0 && t < CSTR.length (instead of t < 3), preserving existing string-lookup fallbacks and return values so negative/float numbers no longer produce undefined array accesses.fixtures/test-figma/community/README.md (1)
14-20: Add a language identifier to the fenced code block.The directory-tree block has no language specified, which triggers a markdownlint warning (MD040). Use
textorplaintextas the language identifier.Proposed fix
-``` +```text <name>.zip └── <name>/ ├── document.json # GET /v1/files/:key?geometry=paths └── images/ └── <ref>.<ext> # Image fills from GET /v1/files/:key/images</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@fixtures/test-figma/community/README.mdaround lines 14 - 20, The fenced
directory-tree code block in the README.md triggers markdownlint MD040 because
it lacks a language identifier; update the triple-backtick fence that wraps the
".zip" tree to include a language tag (e.g., usetext orplaintext)
so the block becomes a labeled plaintext code block and the MD040 warning is
resolved.</details> </blockquote></details> <details> <summary>.tools/figma_archive.py (1)</summary><blockquote> `28-34`: **Consider adding `image/svg+xml` to the MIME map.** Figma image fills can include SVGs. If the Images API ever returns an SVG, the current fallback silently writes it with a `.png` extension. <details> <summary>Proposed fix</summary> ```diff MIME_EXT = { "image/png": "png", "image/jpeg": "jpeg", "image/jpg": "jpg", "image/webp": "webp", "image/gif": "gif", + "image/svg+xml": "svg", } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In @.tools/figma_archive.py around lines 28 - 34, The MIME_EXT mapping is missing SVG support so SVG images from Figma will be saved with a .png fallback; add the "image/svg+xml": "svg" entry to the MIME_EXT dict and verify any code that looks up MIME_EXT (e.g., the logic that determines file extension from a content-type header) uses MIME_EXT.get(content_type, "png") so SVGs get ".svg" instead of defaulting to ".png". ``` </details> </blockquote></details> <details> <summary>crates/grida-canvas-wasm/lib/__test__/environment-node-raster-export.test.ts (1)</summary><blockquote> `148-150`: **Minor: consider narrowing with a type guard instead of repeated casts.** The type assertions on lines 149-150 could be simplified for readability. <details> <summary>Suggested improvement</summary> ```diff const result = canvas.addImageWithId(imageBytes, "res://images/test-fixture-stripes"); expect(result).not.toBe(false); - expect((result as { width: number; height: number }).width).toBeGreaterThan(0); - expect((result as { width: number; height: number }).height).toBeGreaterThan(0); + if (result === false) throw new Error("unreachable"); + expect(result.width).toBeGreaterThan(0); + expect(result.height).toBeGreaterThan(0); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@crates/grida-canvas-wasm/lib/__test__/environment-node-raster-export.test.ts` around lines 148 - 150, The assertions repeatedly cast result to { width: number; height: number }; instead, add a type guard or narrowing check for the result variable (e.g., an isWidthHeight(result) predicate or an inline typeof/instance checks) and then perform width/height assertions without repeated casts; update the test surrounding the result variable and assertions in environment-node-raster-export.test (references: result variable and the width/height expectations) so the value is narrowed once before calling expect(...).toBeGreaterThan(0). ``` </details> </blockquote></details> <details> <summary>editor/grida-canvas-hosted/playground/playground.tsx (1)</summary><blockquote> `475-478`: **`ref` as a variable name is valid but potentially confusing in React code.** While `ref` is not a strict reserved word in JS/TS and works fine as a `const`, it shadows the React concept of `ref` and may confuse readers at a glance. Consider a more descriptive name like `imageRef` or `imageKey`. <details> <summary>Suggested rename</summary> ```diff - const ref = base.includes(".") ? base.split(".")[0]! : base; + const imageRef = base.includes(".") ? base.split(".")[0]! : base; try { - images[ref] = await opfs.readImage(name); + images[imageRef] = await opfs.readImage(name); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@editor/grida-canvas-hosted/playground/playground.tsx` around lines 475 - 478, The variable name ref shadows React's common "ref" concept and can confuse readers; rename the const ref (from the expression const ref = base.includes(".") ? base.split(".")[0]! : base) to a more descriptive identifier like imageKey or imageRef and update all usages (e.g., images[ref] and any references around opfs.readImage(name)) to the new name to preserve behavior while improving clarity. ``` </details> </blockquote></details> <details> <summary>packages/grida-canvas-sdk-render-figma/__tests__/refig.cli.smoke.test.ts (1)</summary><blockquote> `85-89`: **Hardcoded CLI path is brittle.** The path `node_modules/@grida/refig/dist/cli.mjs` will silently break if the package name or dist layout changes. Consider resolving it dynamically, e.g. via `require.resolve` or by reusing the `npx refig` approach (already validated on line 80). <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/__tests__/refig.cli.smoke.test.ts` around lines 85 - 89, The test currently builds a brittle hardcoded path in refigCli and calls execSync with it; change this to resolve the CLI entrypoint dynamically (e.g., use require.resolve to find "@grida/refig/dist/cli.mjs" or reuse the working "npx refig" approach already used on line 80) so the test locates the package regardless of dist layout changes, then pass the resolved path into the execSync call (the same variable refigCli and the execSync invocation should be updated). ``` </details> </blockquote></details> <details> <summary>packages/grida-canvas-sdk-render-figma/__tests__/refig.cli.test.ts (1)</summary><blockquote> `124-124`: **Nit: prefer `path.dirname` over joining with `".."`.** `path.dirname(fullPath)` is more idiomatic and less error-prone than `join(fullPath, "..")`. <details> <summary>Proposed fix</summary> ```diff - mkdirSync(join(fullPath, ".."), { recursive: true }); + mkdirSync(dirname(fullPath), { recursive: true }); ``` (Add `dirname` to the `node:path` import at Line 10.) </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/__tests__/refig.cli.test.ts` at line 124, Replace the join(fullPath, "..") usage with path.dirname(fullPath) to compute the parent directory more idiomatically: update the import to include dirname from "node:path" (alongside existing imports) and change the call site where mkdirSync(join(fullPath, ".."), { recursive: true }) is used to mkdirSync(dirname(fullPath), { recursive: true }); reference symbols: mkdirSync, join, dirname, fullPath, and the node:path import so you add dirname to that import list. ``` </details> </blockquote></details> <details> <summary>packages/grida-canvas-sdk-render-figma/cli.ts (2)</summary><blockquote> `132-169`: **Minor: unnecessary image extraction work for `.fig` input in `runExportAll`.** Lines 152–159 extract images from the `.fig` archive and store them in `rendererOptions.images`. However, looking at `FigmaRenderer.loadScene` (in `lib.ts`, Lines 637–662), the `"fig-file"` path calls `figBytesToSceneJson` which extracts images internally. The `this.options.images` map is only consulted for the `"rest-api-json"` path. Since `document` is constructed from raw bytes (Line 151, `sourceType = "fig-file"`), the extracted images here won't actually be used by the renderer. This doesn't cause incorrect behavior, but it does unnecessary work (parsing zip entries). Consider removing the image extraction for the `.fig` path, or document why it's kept. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/cli.ts` around lines 132 - 169, The code in runExportAll is doing unnecessary image extraction for .fig inputs: remove the fig-file image extraction — delete the iofigma.kiwi.extractImages(...) loop and the images-based assignment to rendererOptions when isFig is true — and ensure rendererOptions remains empty (or only sets loadFigmaDefaultFonts) for the fig path so FigmaRenderer.loadScene/figBytesToSceneJson can handle image extraction internally; keep the existing image-reading branch for the non-.fig path that uses readImagesFromDir. ``` </details> --- `62-100`: **`statSync` on Line 72 throws an unhelpful `ENOENT` error for missing paths.** If the user provides a non-existent input path, `statSync(resolved)` throws a raw Node `ENOENT` error. Wrapping with a user-friendly message would improve CLI UX. <details> <summary>Proposed fix</summary> ```diff const resolved = path.resolve(inputPath); - const stat = statSync(resolved); + if (!existsSync(resolved)) { + throw new Error(`Input path not found: ${resolved}`); + } + const stat = statSync(resolved); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/cli.ts` around lines 62 - 100, The resolveInput function currently calls statSync(resolved) which throws a raw ENOENT for missing paths; wrap that call in a try/catch inside resolveInput, catch the ENOENT (and other FS errors) and rethrow a new Error with a clear CLI-friendly message like "Input path not found: <inputPath>" (include resolved path) so callers get helpful feedback; ensure the rest of resolveInput (directory handling and existsSync checks) remains unchanged and reference resolveInput and the statSync call when making the change. ``` </details> </blockquote></details> <details> <summary>editor/grida-canvas-hosted/playground/uxhost-menu.tsx (1)</summary><blockquote> `191-205`: **Sequential image fetches could be slow with many images.** Each image is fetched one at a time in a `for` loop. For documents with many IMAGE fills, this will be noticeably slow. Consider parallelizing with `Promise.allSettled`. <details> <summary>Proposed parallel fetch</summary> ```diff - const refToBytes: Record<string, Uint8Array> = {}; - for (const ref of imageRefsUsed) { - if (!(ref in images)) continue; - const url = images[ref]; - if (!url) continue; - try { - const resp = await fetch(url); - if (resp.ok) { - const buf = await resp.arrayBuffer(); - refToBytes[ref] = new Uint8Array(buf); - } - } catch (e) { - console.warn(`Failed to fetch image for ref ${ref}`, e); - } - } + const fetchEntries = imageRefsUsed + .filter((ref) => ref in images && images[ref]) + .map(async (ref) => { + try { + const resp = await fetch(images[ref]!); + if (resp.ok) { + const buf = await resp.arrayBuffer(); + return [ref, new Uint8Array(buf)] as const; + } + } catch (e) { + console.warn(`Failed to fetch image for ref ${ref}`, e); + } + return null; + }); + const results = await Promise.allSettled(fetchEntries); + const refToBytes: Record<string, Uint8Array> = {}; + for (const r of results) { + if (r.status === "fulfilled" && r.value) { + refToBytes[r.value[0]] = r.value[1]; + } + } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@editor/grida-canvas-hosted/playground/uxhost-menu.tsx` around lines 191 - 205, The loop that fetches images (iterating imageRefsUsed and checking images[ref]) performs sequential network requests and should be parallelized: build an array of fetch promises for refs that exist in images and have non-empty URLs, use Promise.allSettled to run them concurrently, then iterate the settled results to populate refToBytes (same variable) for fulfilled responses by calling response.arrayBuffer() (or chain that into the promise) and log warnings for rejected items or non-ok responses; keep the existing checks (resp.ok, continue on missing url) and preserve the catch/log behavior but per-promise instead of a top-level try/catch so slow serial waits are avoided. ``` </details> </blockquote></details> <details> <summary>packages/grida-canvas-sdk-render-figma/lib.ts (1)</summary><blockquote> `422-425`: **Static analysis: `forEach` callback should not return a value.** `Set.add()` returns the Set, so the arrow implicitly returns it. Use `for…of` or an explicit block body to silence the lint. <details> <summary>♻️ Proposed fix</summary> ```diff - r.imageRefsUsed.forEach((ref: string) => imageRefsUsed.add(ref)); + for (const ref of r.imageRefsUsed) { + imageRefsUsed.add(ref); + } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/lib.ts` around lines 422 - 425, The forEach callback on r.imageRefsUsed implicitly returns the Set (since Set.add returns the Set) which trips the linter; change the loop to not return a value by replacing r.imageRefsUsed.forEach((ref: string) => imageRefsUsed.add(ref)) with either a for...of loop (for (const ref of r.imageRefsUsed) imageRefsUsed.add(ref)) or an explicit block arrow body (r.imageRefsUsed.forEach((ref: string) => { imageRefsUsed.add(ref); })); update the code surrounding the imageRefsUsed Set and the iteration over individualResults accordingly so the callback no longer returns a value. ``` </details> </blockquote></details> <details> <summary>editor/grida-canvas/editor.ts (1)</summary><blockquote> `3356-3388`: **RID-based image registration looks good; note the dual-key storage.** The `_registerImageWithCustomRid` method stores images with the full RID (`res://images/<ref>`) as the key in `this.images` (line 3375), while `document.images` uses the short `ref` (line 3377). This dual-key scheme is internally consistent — `archivedir()` correctly uses the short ref for document lookups and the full src for WASM byte retrieval — but it makes the `images` Map's key semantics non-obvious. A brief inline comment clarifying that `this.images` is keyed by full RID/URL would help future readers. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@editor/grida-canvas/editor.ts` around lines 3356 - 3388, The _registerImageWithCustomRid method currently stores entries in this.images keyed by the full RID (rid = `res://images/${ref}`) while this.doc.reduce stores them under the short ref; add a concise inline comment in _registerImageWithCustomRid (near the this.images.set(rid, imageRef) line) stating that this.images is intentionally keyed by the full RID/URL (rid) whereas document.images uses the short ref, so future readers understand the dual-keying between this.images and document.images and why archivedir() uses the short ref for lookups and the RID for WASM retrievals. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.tools/figma_archive.py:
- Around line 37-42: The function get_token uses Python 3.10 union syntax "str |
None" which breaks on older Python 3.x; change the annotation to from typing
import Optional and use Optional[str] (or alternatively add "from future
import annotations" at the top) and ensure typing import is added; update the
get_token signature and any other annotations using "|" (e.g.,
get_token(flag_value: Optional[str]) -> str) and keep ENV_TOKEN and error
handling unchanged.In
@editor/grida-canvas/editor.ts:
- Around line 3312-3330: loadImages currently replaces _pending_images when
called before the WASM canvas is ready, losing previously queued images; change
the early-return branch that sets this._pending_images = images to merge new
entries into the existing this._pending_images map instead (preserving earlier
keys) so multiple calls accumulate images; update the branch in loadImages that
checks this.backend === "canvas" && !this._m_wasm_canvas_scene to create
this._pending_images if missing and copy/assign each entry from images into it
(use Object.assign or a simple loop) before returning.- Around line 3296-3305: archivedir() currently calls __parse_image_ref_from_src
which throws for non-res://images/sources causing the archive to crash for
runtime-only sources; modify archivedir() to pre-filter usedSrcs or wrap the
parse call in a safe check/try-catch: skip entries that do not
startWith("res://images/") (or otherwise validate using the same logic as
__parse_image_ref_from_src) and emit a warning/log instead of letting
__parse_image_ref_from_src throw, or catch the Error around the
__parse_image_ref_from_src(...) invocation and continue; reference archivedir()
and __parse_image_ref_from_src for locating the change and consider
DocumentStateQuery.image_srcs() formats when deciding which sources to skip.In
@packages/grida-canvas-io-figma/lib.ts:
- Around line 3137-3146: The Biome linter flags that the concise arrow used in
result.imageRefsUsed.forEach((ref: string) => imageRefsUsed.add(ref)) implicitly
returns a value; change that callback to a block body that calls
imageRefsUsed.add(ref) without returning anything (e.g.
result.imageRefsUsed.forEach((ref: string) => { imageRefsUsed.add(ref); })),
keeping the surrounding merge logic (individualResults.forEach, merged, and
imageRefsUsed) intact so no other behavior changes.In
@packages/grida-canvas-sdk-render-figma/__tests__/refig.test.ts:
- Around line 375-380: The test reads a large fixture zip unconditionally and
will throw ENOENT if the archive is missing; update the test that defines
zipPath/zipBytes so it first checks for the file (e.g. using
fs.existsSync(zipPath)) and if missing logs a console.warn and returns early —
mirror the guard used in the earlier .fig fixture. Locate the zipPath variable
and the readFileSync call in this test and wrap the readFileSync(zipPath) with
the existence check before assigning zipBytes, keeping join and __dirname as-is.- Around line 331-351: The test's strict MIME assertion conflicts with the
allowed PNG fallback; update the assertion in the "renders a REST JSON document
as JPEG" test so it accepts either "image/jpeg" or "image/png" (or alternatively
move the MIME assertion to after expectJpegOrFallbackImage). Locate the test
function (it(... "renders a REST JSON document as JPEG") using the FigmaRenderer
instance and adjust the expect(result.mimeType) check to allow both mime types
(e.g., assert includes "image/jpeg" OR "image/png") so the PNG fallback path in
expectJpegOrFallbackImage isn't precluded.In
@packages/grida-canvas-sdk-render-figma/cli.ts:
- Around line 337-339: The parsed values for width/height/scale (const width =
Number(options.width ?? 1024); const height = Number(options.height ?? 1024);
const scale = Number(options.scale ?? 1);) are not validated and NaN will
silently propagate; after computing these constants validate that width and
height are finite positive integers and scale is a finite positive number (use
Number.isFinite and Number.isInteger or isNaN checks), and if any check fails
log a clear error referencing the offending option (e.g.,
options.width/options.height/options.scale) and abort (throw or process.exit(1))
so the renderer never receives invalid values.- Around line 40-46: The export handling is missing WEBP mappings: update the
formatMap inside exportSettingToRenderOptions to include the "WEBP": "webp"
entry so Figma WEBP exportSettings map to webp render options, and add webp:
"webp" to the EXT_BY_FORMAT constant in cli.ts so file extensions are correct;
modify the formatMap lookup in exportSettingToRenderOptions and the
EXT_BY_FORMAT object accordingly (refer to exportSettingToRenderOptions and
EXT_BY_FORMAT to locate the changes).In
@packages/grida-canvas-sdk-render-figma/lib.ts:
- Around line 343-468: restJsonToSceneJson currently always selects pages[0], so
when a provided rootNodeId lives on another page the function fails or converts
the wrong page; update restJsonToSceneJson to locate the correct page containing
the node (reuse findNodeInRestDocument logic across all pages) when rootNodeId
is provided (or alternatively accept a pageIndex argument) and then operate on
that found page instead of pages[0]; specifically, in restJsonToSceneJson locate
the page that contains the node by iterating doc.document.children and calling
findNodeInRestDocument for each page, throw the existing not-found error only if
no page contains it, and proceed with the existing node packing and context
(functions: restJsonToSceneJson and findNodeInRestDocument).- Around line 139-147: applyScale silently no-ops for exportAs objects that lack
a constraints field (PDF/SVG), so a caller passing scale != 1 gets no feedback;
update applyScale (function applyScale and the types.ExportAs handling) to
detect when constraints is missing and scale !== 1 and emit a diagnostic (e.g.,
console.warn or existing logger) that includes the exportAs.format and requested
scale, then return exportAs unchanged; additionally, add a short note to the
RefigRenderOptions JSDoc documenting that PDF/SVG formats do not support scaling
so callers should expect no scale to be applied.In
@packages/grida-canvas-sdk-render-figma/README.md:
- Around line 308-314: The README's ASCII architecture diagram code block is
missing a language identifier (causing markdownlint MD040); update the fenced
code block that contains the diagram (the block starting withand the lines "Input Conversion Rendering" and ".fig bytes ... REST JSON") to usetext as
the opening fence so it becomestext, leaving the diagram contents unchanged and keeping the closingintact.
Outside diff comments:
In@packages/grida-canvas-io/index.ts:
- Around line 780-798: The ref extraction currently splits the filename on "."
and takes the first segment (in the loop over _x_images), which will truncate
names like "my.image.png"; update the logic that computes base/ref (used where
assets[ref] and imagesRepo[ref] are set) to strip only the final extension
(e.g., use a replace with /.[^.]+$/ or use lastIndexOf to remove the suffix) so
"my.image.png" becomes "my.image" instead of "my"; apply the same fix to the
corresponding logic in archive.pack to keep behavior consistent.
Nitpick comments:
In @.tools/figma_archive.py:
- Around line 28-34: The MIME_EXT mapping is missing SVG support so SVG images
from Figma will be saved with a .png fallback; add the "image/svg+xml": "svg"
entry to the MIME_EXT dict and verify any code that looks up MIME_EXT (e.g., the
logic that determines file extension from a content-type header) uses
MIME_EXT.get(content_type, "png") so SVGs get ".svg" instead of defaulting to
".png".In
@apps/blog/blog/refig/post.md:
- Around line 53-55: The three consecutive bullets starting with "No auth /
fetching", "No design-to-code", and "No editor" feel
repetitive—rephrase one or two to vary the openers while preserving meaning; for
example, change "No auth / fetching" to "Bring your own token storage, HTTP
client, and caching", or change "No design-to-code" to "Rendering only
(pixels / SVG / PDF), not HTML/CSS/Flutter generation", or change "No
editor" to "Read-only: render content without editing"—apply one or two of
these rewordings to the bullets to smooth the rhythm.In
@crates/grida-canvas-wasm/lib/__test__/environment-node-raster-export.test.ts:
- Around line 148-150: The assertions repeatedly cast result to { width: number;
height: number }; instead, add a type guard or narrowing check for the result
variable (e.g., an isWidthHeight(result) predicate or an inline typeof/instance
checks) and then perform width/height assertions without repeated casts; update
the test surrounding the result variable and assertions in
environment-node-raster-export.test (references: result variable and the
width/height expectations) so the value is narrowed once before calling
expect(...).toBeGreaterThan(0).In
@editor/grida-canvas-hosted/playground/playground.tsx:
- Around line 475-478: The variable name ref shadows React's common "ref"
concept and can confuse readers; rename the const ref (from the expression const
ref = base.includes(".") ? base.split(".")[0]! : base) to a more descriptive
identifier like imageKey or imageRef and update all usages (e.g., images[ref]
and any references around opfs.readImage(name)) to the new name to preserve
behavior while improving clarity.In
@editor/grida-canvas-hosted/playground/uxhost-menu.tsx:
- Around line 191-205: The loop that fetches images (iterating imageRefsUsed and
checking images[ref]) performs sequential network requests and should be
parallelized: build an array of fetch promises for refs that exist in images and
have non-empty URLs, use Promise.allSettled to run them concurrently, then
iterate the settled results to populate refToBytes (same variable) for fulfilled
responses by calling response.arrayBuffer() (or chain that into the promise) and
log warnings for rejected items or non-ok responses; keep the existing checks
(resp.ok, continue on missing url) and preserve the catch/log behavior but
per-promise instead of a top-level try/catch so slow serial waits are avoided.In
@editor/grida-canvas-react-starter-kit/starterkit-import/from-figma.tsx:
- Around line 436-445: The current promise chain calls onClose() in .finally(),
which closes the dialog even if the async onImport(r) rejects; move the
onClose() call out of .finally() and invoke it only after successful completion
of onImport — i.e., inside the .then(async (r) => { await onImport?.(r);
onClose(); }) block (or after awaiting onImport in a try block) so that the
dialog remains open when onImport throws, while still allowing errors to
propagate to the .catch handler.In
@editor/grida-canvas/editor.ts:
- Around line 3356-3388: The _registerImageWithCustomRid method currently stores
entries in this.images keyed by the full RID (rid =res://images/${ref}) while
this.doc.reduce stores them under the short ref; add a concise inline comment in
_registerImageWithCustomRid (near the this.images.set(rid, imageRef) line)
stating that this.images is intentionally keyed by the full RID/URL (rid)
whereas document.images uses the short ref, so future readers understand the
dual-keying between this.images and document.images and why archivedir() uses
the short ref for lookups and the RID for WASM retrievals.In
@fixtures/test-figma/community/README.md:
- Around line 14-20: The fenced directory-tree code block in the README.md
triggers markdownlint MD040 because it lacks a language identifier; update the
triple-backtick fence that wraps the ".zip" tree to include a language tag
(e.g., usetext orplaintext) so the block becomes a labeled plaintext
code block and the MD040 warning is resolved.In
@packages/grida-canvas-io-figma/lib.ts:
- Around line 1420-1456: The empty catch in the VECTOR case swallows errors from
restful.map.normalizeRestVectorNetworkToIR and the gridaVectorNetwork
conversion; update that catch to log a warning (e.g., console.warn or
processLogger.warn) including the caught error and contextual identifiers (such
as node.id or gridaId) and a short message indicating vectorNetwork
normalization failed so debugging is possible—apply this change in the catch
following the normalizeRestVectorNetworkToIR call inside the VECTOR case.- Around line 1949-1996: The fmt numeric branch is too permissive and can index
FMT with negative or non-integer values; update the numeric checks in
kiwi_has_export_settings_trait so fmt uses Number.isInteger(t) && t >= 0 && t <
FMT.length (instead of t < 4) and similarly make cstr use Number.isInteger(t) &&
t >= 0 && t < CSTR.length (instead of t < 3), preserving existing string-lookup
fallbacks and return values so negative/float numbers no longer produce
undefined array accesses.In
@packages/grida-canvas-io/__tests__/format-roundtrip.test.ts:
- Around line 2424-2473: The stroke ImagePaint roundtrip test ("roundtrips
ImagePaint stroke_paints on ContainerNode") is missing an assertion for
paint.src; update the assertion block inside the roundtripTest callback to
verify the image source survived the roundtrip by asserting paint.src equals
"res://images/fedcba9876543210" (use the same pattern as the fill-paint
counterpart and the existing checks for paint.fit, paint.blend_mode,
paint.opacity, paint.active, and paint.filters).In
@packages/grida-canvas-sdk-render-figma/__tests__/refig.cli.smoke.test.ts:
- Around line 85-89: The test currently builds a brittle hardcoded path in
refigCli and calls execSync with it; change this to resolve the CLI entrypoint
dynamically (e.g., use require.resolve to find "@grida/refig/dist/cli.mjs" or
reuse the working "npx refig" approach already used on line 80) so the test
locates the package regardless of dist layout changes, then pass the resolved
path into the execSync call (the same variable refigCli and the execSync
invocation should be updated).In
@packages/grida-canvas-sdk-render-figma/__tests__/refig.cli.test.ts:
- Line 124: Replace the join(fullPath, "..") usage with path.dirname(fullPath)
to compute the parent directory more idiomatically: update the import to include
dirname from "node:path" (alongside existing imports) and change the call site
where mkdirSync(join(fullPath, ".."), { recursive: true }) is used to
mkdirSync(dirname(fullPath), { recursive: true }); reference symbols: mkdirSync,
join, dirname, fullPath, and the node:path import so you add dirname to that
import list.In
@packages/grida-canvas-sdk-render-figma/__tests__/refig.test.ts:
- Around line 353-373: The test for scaling using FigmaRenderer.render currently
only checks the PNG signature; update the "renders with scale option" test to
also assert that result.width and result.height reflect the scale (i.e., width
=== 256 * scale and height === 256 * scale when calling renderer.render("1:1", {
width: 256, height: 256, scale: 2 })); locate the test case using the
FigmaRenderer instance and the render invocation to add the two expect
assertions for result.width and result.height before writing the file and
disposing the renderer.In
@packages/grida-canvas-sdk-render-figma/cli.ts:
- Around line 132-169: The code in runExportAll is doing unnecessary image
extraction for .fig inputs: remove the fig-file image extraction — delete the
iofigma.kiwi.extractImages(...) loop and the images-based assignment to
rendererOptions when isFig is true — and ensure rendererOptions remains empty
(or only sets loadFigmaDefaultFonts) for the fig path so
FigmaRenderer.loadScene/figBytesToSceneJson can handle image extraction
internally; keep the existing image-reading branch for the non-.fig path that
uses readImagesFromDir.- Around line 62-100: The resolveInput function currently calls
statSync(resolved) which throws a raw ENOENT for missing paths; wrap that call
in a try/catch inside resolveInput, catch the ENOENT (and other FS errors) and
rethrow a new Error with a clear CLI-friendly message like "Input path not
found: " (include resolved path) so callers get helpful feedback;
ensure the rest of resolveInput (directory handling and existsSync checks)
remains unchanged and reference resolveInput and the statSync call when making
the change.In
@packages/grida-canvas-sdk-render-figma/lib.ts:
- Around line 422-425: The forEach callback on r.imageRefsUsed implicitly
returns the Set (since Set.add returns the Set) which trips the linter; change
the loop to not return a value by replacing r.imageRefsUsed.forEach((ref:
string) => imageRefsUsed.add(ref)) with either a for...of loop (for (const ref
of r.imageRefsUsed) imageRefsUsed.add(ref)) or an explicit block arrow body
(r.imageRefsUsed.forEach((ref: string) => { imageRefsUsed.add(ref); })); update
the code surrounding the imageRefsUsed Set and the iteration over
individualResults accordingly so the callback no longer returns a value.</details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+37
to
+42
| def get_token(flag_value: str | None) -> str: | ||
| token = flag_value or os.environ.get(ENV_TOKEN) | ||
| if not token or not token.strip(): | ||
| print(f"error: provide --x-figma-token or set {ENV_TOKEN}", file=sys.stderr) | ||
| sys.exit(1) | ||
| return token.strip() |
There was a problem hiding this comment.
str | None union syntax requires Python ≥ 3.10.
If this script needs to run on older Python 3 versions (e.g., 3.8/3.9), use Optional[str] from typing instead or add from __future__ import annotations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.tools/figma_archive.py around lines 37 - 42, The function get_token uses
Python 3.10 union syntax "str | None" which breaks on older Python 3.x; change
the annotation to from typing import Optional and use Optional[str] (or
alternatively add "from __future__ import annotations" at the top) and ensure
typing import is added; update the get_token signature and any other annotations
using "|" (e.g., get_token(flag_value: Optional[str]) -> str) and keep ENV_TOKEN
and error handling unchanged.
Comment on lines
+3296
to
3305
| private static __parse_image_ref_from_src(src: string): string { | ||
| const raw = src.trim(); | ||
| if ( | ||
| raw.startsWith("res://images/") && | ||
| raw.length > "res://images/".length | ||
| ) { | ||
| return raw.slice("res://images/".length); | ||
| } | ||
| return /^[0-9a-f]{16}$/.test(raw) ? raw : null; | ||
| throw new Error(`Image src must be res://images/<ref>. Got: "${src}"`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n '__parse_image_ref_from_src' editor/grida-canvas/editor.ts -C3Repository: gridaco/grida
Length of output: 552
🏁 Script executed:
sed -n '2890,2920p' editor/grida-canvas/editor.tsRepository: gridaco/grida
Length of output: 1082
🏁 Script executed:
# Check all call sites of __parse_image_ref_from_src in the entire repo
rg -n '__parse_image_ref_from_src' --type=ts
# Also search for DocumentStateQuery definition
rg -n 'image_srcs' --type=ts -C2Repository: gridaco/grida
Length of output: 967
🏁 Script executed:
sed -n '658,680p' editor/grida-canvas/query/index.tsRepository: gridaco/grida
Length of output: 411
🏁 Script executed:
sed -n '650,660p' editor/grida-canvas/query/index.tsRepository: gridaco/grida
Length of output: 308
🏁 Script executed:
rg -n 'image_paints' editor/grida-canvas/query/index.ts -C1 -A10 | head -50Repository: gridaco/grida
Length of output: 628
🏁 Script executed:
sed -n '622,660p' editor/grida-canvas/query/index.tsRepository: gridaco/grida
Length of output: 1073
🏁 Script executed:
sed -n '2920,2940p' editor/grida-canvas/editor.tsRepository: gridaco/grida
Length of output: 580
🏁 Script executed:
# Search for how paint.src is set or what values it can have
rg -n 'paint\.src' editor/grida-canvas/ --type=ts -B2 -A2 | head -80Repository: gridaco/grida
Length of output: 374
🏁 Script executed:
# Search for ImagePaint type definition and its src property
rg -n 'ImagePaint' editor/grida-canvas/ --type=ts | head -20Repository: gridaco/grida
Length of output: 1659
🏁 Script executed:
# Search for where src property is assigned on ImagePaint or where images come from
rg -n '\.src\s*=' editor/grida-canvas/ --type=ts | grep -i paint | head -20Repository: gridaco/grida
Length of output: 39
🏁 Script executed:
# Look for where image src values are set or imported from user content
rg -n 'http' editor/grida-canvas/editor.ts | head -20Repository: gridaco/grida
Length of output: 145
🏁 Script executed:
# Search for where images are added to document or paint.src is set
rg -n 'blob:|mem://' editor/grida-canvas/ --type=ts | head -20Repository: gridaco/grida
Length of output: 146
🏁 Script executed:
# Search for DOM backend and image handling
rg -n 'DOM|dom' editor/grida-canvas/editor.ts | head -20Repository: gridaco/grida
Length of output: 528
🏁 Script executed:
sed -n '2875,2910p' editor/grida-canvas/editor.tsRepository: gridaco/grida
Length of output: 1411
🏁 Script executed:
# Search for where paint.src is set to res:// format
rg -n 'res://images/' editor/grida-canvas/ --type=ts | head -30Repository: gridaco/grida
Length of output: 887
Fix line number and verify unguarded throw path in archivedir().
The method is called at line 2904 (not 904), not 904, within archivedir()'s loop over usedSrcs. The backend guard (lines 2897–2899) checks canvas/WASM availability but does not validate src format. Since DocumentStateQuery.image_srcs() docstring indicates paint sources can include runtime formats like blob: or mem:// (line 658 in query/index.ts), non-res://images/ srcs on a canvas backend document would cause an unrecoverable throw mid-archive. Add a pre-filter or softer failure (skip + warn) before the parse call.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor/grida-canvas/editor.ts` around lines 3296 - 3305, archivedir()
currently calls __parse_image_ref_from_src which throws for non-`res://images/`
sources causing the archive to crash for runtime-only sources; modify
archivedir() to pre-filter usedSrcs or wrap the parse call in a safe
check/try-catch: skip entries that do not startWith("res://images/") (or
otherwise validate using the same logic as __parse_image_ref_from_src) and emit
a warning/log instead of letting __parse_image_ref_from_src throw, or catch the
Error around the __parse_image_ref_from_src(...) invocation and continue;
reference archivedir() and __parse_image_ref_from_src for locating the change
and consider DocumentStateQuery.image_srcs() formats when deciding which sources
to skip.
Comment on lines
+3137
to
3146
| individualResults.forEach((result) => { | ||
| const doc = result.document; | ||
| Object.assign(merged.nodes, doc.nodes); | ||
| Object.assign(merged.links, doc.links); | ||
| Object.assign(merged.images, doc.images); | ||
| Object.assign(merged.bitmaps, doc.bitmaps); | ||
| Object.assign(merged.properties, doc.properties); | ||
| merged.scene.children_refs.push(...doc.scene.children_refs); | ||
| result.imageRefsUsed.forEach((ref: string) => imageRefsUsed.add(ref)); | ||
| }); |
There was a problem hiding this comment.
forEach callback should not return a value (Biome lint).
Set.prototype.add returns the Set, so the arrow in result.imageRefsUsed.forEach((ref: string) => imageRefsUsed.add(ref)) implicitly returns a value. Use a block body to satisfy the linter.
Suggested fix
- result.imageRefsUsed.forEach((ref: string) => imageRefsUsed.add(ref));
+ result.imageRefsUsed.forEach((ref: string) => {
+ imageRefsUsed.add(ref);
+ });📝 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
| individualResults.forEach((result) => { | |
| const doc = result.document; | |
| Object.assign(merged.nodes, doc.nodes); | |
| Object.assign(merged.links, doc.links); | |
| Object.assign(merged.images, doc.images); | |
| Object.assign(merged.bitmaps, doc.bitmaps); | |
| Object.assign(merged.properties, doc.properties); | |
| merged.scene.children_refs.push(...doc.scene.children_refs); | |
| result.imageRefsUsed.forEach((ref: string) => imageRefsUsed.add(ref)); | |
| }); | |
| individualResults.forEach((result) => { | |
| const doc = result.document; | |
| Object.assign(merged.nodes, doc.nodes); | |
| Object.assign(merged.links, doc.links); | |
| Object.assign(merged.images, doc.images); | |
| Object.assign(merged.bitmaps, doc.bitmaps); | |
| Object.assign(merged.properties, doc.properties); | |
| merged.scene.children_refs.push(...doc.scene.children_refs); | |
| result.imageRefsUsed.forEach((ref: string) => { | |
| imageRefsUsed.add(ref); | |
| }); | |
| }); |
🧰 Tools
🪛 Biome (2.3.14)
[error] 3145-3145: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/grida-canvas-io-figma/lib.ts` around lines 3137 - 3146, The Biome
linter flags that the concise arrow used in result.imageRefsUsed.forEach((ref:
string) => imageRefsUsed.add(ref)) implicitly returns a value; change that
callback to a block body that calls imageRefsUsed.add(ref) without returning
anything (e.g. result.imageRefsUsed.forEach((ref: string) => {
imageRefsUsed.add(ref); })), keeping the surrounding merge logic
(individualResults.forEach, merged, and imageRefsUsed) intact so no other
behavior changes.
Comment on lines
+331
to
+351
| it("renders a REST JSON document as JPEG", async () => { | ||
| const renderer = new FigmaRenderer(MINIMAL_REST_FIXTURE, { | ||
| loadFigmaDefaultFonts: false, | ||
| }); | ||
|
|
||
| try { | ||
| const result = await renderer.render("1:1", { | ||
| format: "jpeg", | ||
| width: 256, | ||
| height: 256, | ||
| }); | ||
|
|
||
| expect(result.mimeType).toBe("image/jpeg"); | ||
| expectJpegOrFallbackImage(result.data); | ||
|
|
||
| const outPath = join(TEST_OUTPUT_DIR, "rest-frame.jpeg"); | ||
| writeFileSync(outPath, Buffer.from(result.data)); | ||
| } finally { | ||
| renderer.dispose(); | ||
| } | ||
| }, 30_000); |
There was a problem hiding this comment.
JPEG mimeType assertion conflicts with the PNG fallback accepted by expectJpegOrFallbackImage.
Line 343 asserts result.mimeType is "image/jpeg", but expectJpegOrFallbackImage (line 344) explicitly tolerates a PNG fallback when WASM Skia lacks JPEG encoding. If the fallback triggers, the strict mimeType assertion on line 343 will fail before the lenient binary check runs, making the fallback tolerance dead code.
Either relax the mimeType assertion to accept both, or move it after the binary check:
Proposed fix
- expect(result.mimeType).toBe("image/jpeg");
- expectJpegOrFallbackImage(result.data);
+ expectJpegOrFallbackImage(result.data);
+ // mimeType may report "image/jpeg" even when Skia falls back to PNG encoding
+ expect(["image/jpeg", "image/png"]).toContain(result.mimeType);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/grida-canvas-sdk-render-figma/__tests__/refig.test.ts` around lines
331 - 351, The test's strict MIME assertion conflicts with the allowed PNG
fallback; update the assertion in the "renders a REST JSON document as JPEG"
test so it accepts either "image/jpeg" or "image/png" (or alternatively move the
MIME assertion to after expectJpegOrFallbackImage). Locate the test function
(it(... "renders a REST JSON document as JPEG") using the FigmaRenderer instance
and adjust the expect(result.mimeType) check to allow both mime types (e.g.,
assert includes "image/jpeg" OR "image/png") so the PNG fallback path in
expectJpegOrFallbackImage isn't precluded.
Comment on lines
+40
to
+46
| const EXT_BY_FORMAT: Record<string, string> = { | ||
| png: "png", | ||
| jpeg: "jpeg", | ||
| jpg: "jpeg", | ||
| svg: "svg", | ||
| pdf: "pdf", | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if "webp" format is referenced or produced in the codebase
echo "=== Searching for webp references ==="
rg -i 'webp' packages/grida-canvas-sdk-render-figma/ -n
echo -e "\n=== Searching for FORMAT_SET or format definitions ==="
rg 'FORMAT_SET|formatMap|export.*format' packages/grida-canvas-sdk-render-figma/ -n -A 2
echo -e "\n=== Checking exportSettingToRenderOptions implementation ==="
rg -n 'exportSettingToRenderOptions' packages/grida-canvas-sdk-render-figma/ -B 2 -A 15Repository: gridaco/grida
Length of output: 19475
🏁 Script executed:
#!/bin/bash
# Check the structure of the cli.ts file to understand format handling better
echo "=== Examining the cli.ts file structure ==="
head -n 50 packages/grida-canvas-sdk-render-figma/cli.ts | cat -n
echo -e "\n=== Checking the exportAllOutputBasename function ==="
sed -n '120,130p' packages/grida-canvas-sdk-render-figma/cli.ts | cat -nRepository: gridaco/grida
Length of output: 2152
🏁 Script executed:
#!/bin/bash
# Check if there are any tests or references to webp export
echo "=== Searching for webp in tests or examples ==="
find packages/grida-canvas-sdk-render-figma -name "*.test.*" -o -name "*.spec.*" | xargs grep -l webp 2>/dev/null
echo -e "\n=== Check the full cli.ts to understand format flow ==="
wc -l packages/grida-canvas-sdk-render-figma/cli.tsRepository: gridaco/grida
Length of output: 211
EXT_BY_FORMAT and exportSettingToRenderOptions both missing webp support — WEBP export settings from Figma convert to png.
When a Figma node has a WEBP export setting, exportSettingToRenderOptions (lib.ts, line 238) falls back to "png" because the formatMap is missing the "WEBP": "webp" entry. This causes the entire export to be rendered as PNG—not just the filename extension. Additionally, EXT_BY_FORMAT in cli.ts is missing the "webp": "webp" entry, causing the fallback file extension to be wrong.
Both must be fixed:
- Add
"WEBP": "webp"toformatMapinexportSettingToRenderOptions(lib.ts, ~line 232) - Add
webp: "webp"toEXT_BY_FORMAT(cli.ts, line 40)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/grida-canvas-sdk-render-figma/cli.ts` around lines 40 - 46, The
export handling is missing WEBP mappings: update the formatMap inside
exportSettingToRenderOptions to include the "WEBP": "webp" entry so Figma WEBP
exportSettings map to webp render options, and add webp: "webp" to the
EXT_BY_FORMAT constant in cli.ts so file extensions are correct; modify the
formatMap lookup in exportSettingToRenderOptions and the EXT_BY_FORMAT object
accordingly (refer to exportSettingToRenderOptions and EXT_BY_FORMAT to locate
the changes).
Comment on lines
+337
to
+339
| const width = Number(options.width ?? 1024); | ||
| const height = Number(options.height ?? 1024); | ||
| const scale = Number(options.scale ?? 1); |
There was a problem hiding this comment.
No validation for non-numeric --width, --height, --scale values.
If a user passes --width foo, Number("foo") produces NaN, which silently propagates to the renderer. Consider validating these after parsing.
Proposed fix
const width = Number(options.width ?? 1024);
const height = Number(options.height ?? 1024);
const scale = Number(options.scale ?? 1);
+ if (Number.isNaN(width) || Number.isNaN(height) || Number.isNaN(scale)) {
+ program.error("--width, --height, and --scale must be valid numbers");
+ }
await runSingleNode(documentPath, nodeId, path.resolve(outPath), {📝 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 width = Number(options.width ?? 1024); | |
| const height = Number(options.height ?? 1024); | |
| const scale = Number(options.scale ?? 1); | |
| const width = Number(options.width ?? 1024); | |
| const height = Number(options.height ?? 1024); | |
| const scale = Number(options.scale ?? 1); | |
| if (Number.isNaN(width) || Number.isNaN(height) || Number.isNaN(scale)) { | |
| program.error("--width, --height, and --scale must be valid numbers"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/grida-canvas-sdk-render-figma/cli.ts` around lines 337 - 339, The
parsed values for width/height/scale (const width = Number(options.width ??
1024); const height = Number(options.height ?? 1024); const scale =
Number(options.scale ?? 1);) are not validated and NaN will silently propagate;
after computing these constants validate that width and height are finite
positive integers and scale is a finite positive number (use Number.isFinite and
Number.isInteger or isNaN checks), and if any check fails log a clear error
referencing the offending option (e.g.,
options.width/options.height/options.scale) and abort (throw or process.exit(1))
so the renderer never receives invalid values.
Comment on lines
+139
to
+147
| function applyScale(exportAs: types.ExportAs, scale: number): types.ExportAs { | ||
| if ("constraints" in exportAs) { | ||
| return { | ||
| ...exportAs, | ||
| constraints: { type: "scale", value: scale }, | ||
| } as types.ExportAs; | ||
| } | ||
| return exportAs; | ||
| } |
There was a problem hiding this comment.
applyScale silently no-ops for PDF and SVG formats.
mapFormat returns objects without a constraints field for PDF ({ format: "PDF" }) and SVG ({ format: "SVG" }), so applyScale returns them unchanged. A user passing scale: 2 with format: "pdf" gets no scaling and no warning. Consider either logging a diagnostic or documenting this limitation in the RefigRenderOptions JSDoc.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/grida-canvas-sdk-render-figma/lib.ts` around lines 139 - 147,
applyScale silently no-ops for exportAs objects that lack a constraints field
(PDF/SVG), so a caller passing scale != 1 gets no feedback; update applyScale
(function applyScale and the types.ExportAs handling) to detect when constraints
is missing and scale !== 1 and emit a diagnostic (e.g., console.warn or existing
logger) that includes the exportAs.format and requested scale, then return
exportAs unchanged; additionally, add a short note to the RefigRenderOptions
JSDoc documenting that PDF/SVG formats do not support scaling so callers should
expect no scale to be applied.
Comment on lines
+343
to
+468
| function restJsonToSceneJson( | ||
| json: FigmaJsonDocument, | ||
| rootNodeId?: string, | ||
| images?: Record<string, Uint8Array> | ||
| ): RestConversionResult { | ||
| const doc = json as { | ||
| document?: { children?: Array<Record<string, unknown>> }; | ||
| }; | ||
| const pages = doc?.document?.children; | ||
| if (!pages || pages.length === 0) { | ||
| throw new Error("FigmaDocument: REST JSON has no document pages"); | ||
| } | ||
|
|
||
| const page = pages[0] as { | ||
| name?: string; | ||
| children?: Array<Record<string, unknown>>; | ||
| }; | ||
| const rootNodes = page.children ?? []; | ||
| if (rootNodes.length === 0) { | ||
| throw new Error("FigmaDocument: first page has no root nodes"); | ||
| } | ||
|
|
||
| let counter = 0; | ||
| const baseGradientGen = () => `grad-${++counter}`; | ||
|
|
||
| const resolveImageSrc = | ||
| images && | ||
| (Object.keys(images).length > 0 | ||
| ? (ref: string) => (ref in images ? `res://images/${ref}` : null) | ||
| : undefined); | ||
|
|
||
| const buildContext = ( | ||
| overrides: Partial<iofigma.restful.factory.FactoryContext> | ||
| ): iofigma.restful.factory.FactoryContext => ({ | ||
| gradient_id_generator: baseGradientGen, | ||
| ...(resolveImageSrc && { resolve_image_src: resolveImageSrc }), | ||
| ...overrides, | ||
| }); | ||
|
|
||
| if (rootNodeId != null && rootNodeId !== "") { | ||
| const node = findNodeInRestDocument(json, rootNodeId); | ||
| if (!node) { | ||
| throw new Error( | ||
| `FigmaDocument: node with id "${rootNodeId}" not found in document` | ||
| ); | ||
| } | ||
| let rootIdUsed = false; | ||
| const context = buildContext({ | ||
| node_id_generator: () => { | ||
| if (!rootIdUsed) { | ||
| rootIdUsed = true; | ||
| return rootNodeId; | ||
| } | ||
| return `refig-${++counter}`; | ||
| }, | ||
| }); | ||
| const { document: packed, imageRefsUsed } = | ||
| iofigma.restful.factory.document(node as any, {}, context); | ||
| const fullDoc = | ||
| grida.program.nodes.factory.packed_scene_document_to_full_document( | ||
| packed | ||
| ); | ||
| return { | ||
| sceneJson: JSON.stringify({ | ||
| version: grida.program.document.SCHEMA_VERSION, | ||
| document: fullDoc, | ||
| }), | ||
| imageRefsUsed, | ||
| }; | ||
| } | ||
|
|
||
| const context = buildContext({ | ||
| node_id_generator: () => `refig-${++counter}`, | ||
| }); | ||
|
|
||
| const individualResults = rootNodes.map((rootNode) => | ||
| iofigma.restful.factory.document(rootNode as any, {}, context) | ||
| ); | ||
|
|
||
| const imageRefsUsed = new Set<string>(); | ||
| for (const r of individualResults) { | ||
| r.imageRefsUsed.forEach((ref: string) => imageRefsUsed.add(ref)); | ||
| } | ||
|
|
||
| let packed: grida.program.document.IPackedSceneDocument; | ||
| if (individualResults.length === 1) { | ||
| packed = individualResults[0].document; | ||
| } else { | ||
| packed = { | ||
| bitmaps: {}, | ||
| images: {}, | ||
| nodes: {}, | ||
| links: {}, | ||
| properties: {}, | ||
| scene: { | ||
| type: "scene", | ||
| id: "main", | ||
| name: page.name ?? "Page 1", | ||
| children_refs: [], | ||
| guides: [], | ||
| edges: [], | ||
| constraints: { children: "multiple" }, | ||
| }, | ||
| }; | ||
| for (const result of individualResults) { | ||
| const d = result.document; | ||
| Object.assign(packed.nodes, d.nodes); | ||
| Object.assign(packed.links, d.links); | ||
| Object.assign(packed.images, d.images); | ||
| Object.assign(packed.bitmaps, d.bitmaps); | ||
| Object.assign(packed.properties, d.properties); | ||
| packed.scene.children_refs.push(...d.scene.children_refs); | ||
| } | ||
| } | ||
|
|
||
| const fullDoc = | ||
| grida.program.nodes.factory.packed_scene_document_to_full_document(packed); | ||
|
|
||
| return { | ||
| sceneJson: JSON.stringify({ | ||
| version: grida.program.document.SCHEMA_VERSION, | ||
| document: fullDoc, | ||
| }), | ||
| imageRefsUsed: Array.from(imageRefsUsed), | ||
| }; | ||
| } |
There was a problem hiding this comment.
restJsonToSceneJson always uses the first page — consider documenting or accepting a page index.
Line 356 unconditionally takes pages[0]. For multi-page Figma documents where the target rootNodeId lives on a page other than the first, the conversion either fails (node not found) or silently converts only the first page. The .fig path (figBytesToSceneJson) already handles this by finding the page containing the node — REST path could benefit from the same logic.
🧰 Tools
🪛 Biome (2.3.14)
[error] 424-424: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/grida-canvas-sdk-render-figma/lib.ts` around lines 343 - 468,
restJsonToSceneJson currently always selects pages[0], so when a provided
rootNodeId lives on another page the function fails or converts the wrong page;
update restJsonToSceneJson to locate the correct page containing the node (reuse
findNodeInRestDocument logic across all pages) when rootNodeId is provided (or
alternatively accept a pageIndex argument) and then operate on that found page
instead of pages[0]; specifically, in restJsonToSceneJson locate the page that
contains the node by iterating doc.document.children and calling
findNodeInRestDocument for each page, throw the existing not-found error only if
no page contains it, and proceed with the existing node packing and context
(functions: restJsonToSceneJson and findNodeInRestDocument).
Comment on lines
+308
to
+314
| ``` | ||
| Input Conversion Rendering | ||
| ───── ────────── ───────── | ||
| .fig bytes ──┐ | ||
| ├──→ @grida/io-figma ──→ Grida IR ──→ @grida/canvas-wasm ──→ PNG/JPEG/WebP/PDF/SVG | ||
| REST JSON ───┘ | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the architecture diagram code block.
Flagged by markdownlint (MD040). Use ```text for ASCII diagrams.
Proposed fix
-```
+```text
Input Conversion Rendering
───── ────────── ─────────
.fig bytes ──┐
├──→ `@grida/io-figma` ──→ Grida IR ──→ `@grida/canvas-wasm` ──→ PNG/JPEG/WebP/PDF/SVG
REST JSON ───┘
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 308-308: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/grida-canvas-sdk-render-figma/README.md` around lines 308 - 314, The
README's ASCII architecture diagram code block is missing a language identifier
(causing markdownlint MD040); update the fenced code block that contains the
diagram (the block starting with ``` and the lines "Input Conversion Rendering"
and ".fig bytes ... REST JSON") to use ```text as the opening fence so it
becomes ```text, leaving the diagram contents unchanged and keeping the closing
``` intact.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
packages/grida-canvas-sdk-render-figma/cli.ts (2)
63-101: File-type detection is duplicated acrossresolveInput,runExportAll, andrunSingleNode.
resolveInputdeterminesisRestJsonby extension/convention, but neitherrunExportAllnorrunSingleNodereceives or uses that flag. Each re-derives the file type independently (isFigon line 139,isJsonon line 220). This is fragile — if the detection heuristic evolves (e.g., sniffing magic bytes), it must be updated in three places.Consider threading the
isRestJsonflag fromresolveInputinto both runner functions.Also applies to: 133-170, 202-234
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/cli.ts` around lines 63 - 101, The file-type detection is duplicated; stop re-deriving it in runExportAll and runSingleNode by passing the isRestJson result from resolveInput into those functions and using that flag instead of their local isFig/isJson heuristics. Update resolveInput to return isRestJson (already present), change the caller that invokes runExportAll and runSingleNode to forward the returned isRestJson value, and remove or replace the internal detection logic in runExportAll and runSingleNode so they rely solely on the passed isRestJson flag (keep function names runExportAll and runSingleNode unchanged to locate the edits).
133-200:.figexport-all parses the file twice (once for export collection, once per render).In
runExportAll, the.figbranch parses the file at line 149 (iofigma.kiwi.parseFile) to collect export items, then creates aFigmaDocumentwith the raw bytes. Whenrenderer.render()is called,loadScene→figBytesToSceneJsonparses the same bytes again (and again on every node-id change). For a file with many export settings this is unnecessarily expensive.Consider passing the already-parsed
figFilethrough to the renderer, or caching the parse result as suggested in the lib.ts comment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/cli.ts` around lines 133 - 200, runExportAll parses .fig bytes twice: once with iofigma.kiwi.parseFile to collect exports and again inside the renderer during render/loadScene; to fix, propagate the already-parsed figFile into the rendering path instead of reparsing on each render. Specifically, in runExportAll (where figFile is created and images are extracted) attach the parsed figFile to the FigmaDocument or include it in rendererOptions (e.g., rendererOptions.figFile or document.parsedFig = figFile) and update FigmaRenderer (and its loadScene/figBytesToSceneJson usage) to check for and use this parsed figFile before attempting to parse bytes again, so subsequent calls to render(nid, ...) reuse the cached parse.packages/grida-canvas-sdk-render-figma/lib.ts (2)
423-426: Biome lint:forEachcallback should not return a value.
Set.prototype.add()returns theSet, so the arrow in theforEachimplicitly returns it. While harmless at runtime, it triggerslint/suspicious/useIterableCallbackReturn. Use a block body or afor…ofloop.♻️ Proposed fix
- for (const r of individualResults) { - r.imageRefsUsed.forEach((ref: string) => imageRefsUsed.add(ref)); - } + for (const r of individualResults) { + for (const ref of r.imageRefsUsed) { + imageRefsUsed.add(ref); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/lib.ts` around lines 423 - 426, The current code uses r.imageRefsUsed.forEach((ref: string) => imageRefsUsed.add(ref)) which implicitly returns the Set and triggers the lint rule; update the loop to avoid returning a value by replacing the inner forEach with either a block arrow or a for...of loop — e.g., iterate over r.imageRefsUsed with for (const ref of r.imageRefsUsed) { imageRefsUsed.add(ref); } ensuring you reference the existing variables imageRefsUsed, individualResults, and r.imageRefsUsed (and the Set.prototype.add call) so no implicit return occurs.
636-671: Re-parsing.figbytes on everynodeIdchange is expensive.
loadScenecallsfigBytesToSceneJsonwhich callsiofigma.kiwi.parseFile(figBytes)each time the requested node changes. For--export-allflows that render many nodes, this re-parses the entire binary and re-extracts images on every iteration. Consider caching the parse result (figFile) and the extracted images after the first load.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/lib.ts` around lines 636 - 671, loadScene currently re-parses .fig bytes on every nodeId by calling figBytesToSceneJson (which invokes iofigma.kiwi.parseFile); to fix, add a cache on the instance (e.g., _figFileCache and _figImagesCache) to store the parsed figFile and extracted images after the first figBytesToSceneJson call and reuse them for subsequent loadScene calls for the same document payload, falling back to calling figBytesToSceneJson only when there is no cache or when document.payload changes; update loadScene to check the cache before calling figBytesToSceneJson, populate the cache with figResult.sceneJson and figResult.images (or store parsed figFile and reuse rest of conversion), and ensure cache invalidation when the document payload or sourceType changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@editor/grida-canvas/editor.ts`:
- Around line 3359-3391: _registerImageWithCustomRid currently keys this.images
with the full RID while state.document.images uses the bare ref, causing lookup
mismatch; change _registerImageWithCustomRid to keep using the constructed rid
for the ImageRef.url but use the bare ref as the Map key (call
this.images.set(ref, imageRef) instead of this.images.set(rid, ...)), ensure any
other runtime helpers that check or fetch images (__is_image_registered,
__get_image_ref or similar) continue to use the bare ref for lookups, and only
construct the full rid (`res://images/${ref}`) when calling the WASM API
addImageWithId or when serializing the ImageRef.url. Ensure the stored
imageRef.url remains the full RID but all in-memory Map keys match
document.images' keys (the bare ref).
In `@packages/grida-canvas-sdk-render-figma/cli.ts`:
- Around line 275-278: Update the CLI help text for the "--export-all" option to
accurately state supported formats by removing the "(REST JSON only)" qualifier
and noting that .fig files are supported; change the description used where
.option("--export-all", "Export every node that has exportSettings (REST JSON
only)") is declared so it reflects that runExportAll handles both REST JSON and
.fig file paths (see runExportAll function for the .fig code path that parses
and builds a REST-like document).
In `@packages/grida-canvas-sdk-render-figma/lib.ts`:
- Around line 428-456: The merge using Object.assign into
packed.nodes/links/images/bitmaps/properties while iterating individualResults
can silently overwrite entries with colliding ids; update the merge logic in the
block that builds packed (the packed variable and loop over individualResults)
to detect key collisions before assigning (e.g., iterate d.nodes keys and if
packed.nodes[key] exists either throw/log error or generate a stable unique key
and remap references) and apply the same collision-checking/remapping for links,
images, bitmaps and properties so no entries are silently dropped and
scene.children_refs are updated to the remapped ids if renaming is used.
- Around line 617-634: ensureCanvas currently caches this._canvas and ignores
width/height on subsequent calls, causing renders after the first to use the
stale viewport; update ensureCanvas to detect size changes and recreate the
canvas (call createCanvas again) when width or height differ from the existing
canvas, or alternatively add a public resize method on the Canvas wrapper that
delegates to the underlying Scene.resize so ensureCanvas can call that;
specifically modify ensureCanvas (and/or the Canvas wrapper class) to compare
requested width/height with the existing canvas dimensions and either recreate
via createCanvas or call Scene.resize so subsequent render() calls honor new
sizes, and keep the existing logic that still applies ensureFigmaDefaultFonts
when loadFigmaDefaultFonts is enabled.
---
Duplicate comments:
In `@editor/grida-canvas/editor.ts`:
- Around line 2903-2914: The loop in archivedir() calls
Editor.__parse_image_ref_from_src for every source returned by
DocumentStateQuery.image_srcs(), which can throw on runtime-only sources (e.g.
blob:, mem:) and abort the whole archive; filter or guard those sources first
(e.g. only process sources that match the res://images/ pattern) or wrap the
parse/get logic in a try/catch and continue on error; update the loop to skip
non-res image sources before calling Editor.__parse_image_ref_from_src and
__get_image_bytes_for_wasm (and still handle missing bytes via the existing
check against snapshot.images).
In `@packages/grida-canvas-sdk-render-figma/cli.ts`:
- Around line 338-340: The CLI reads options.width/options.height/options.scale
into width/height/scale without validating non-numeric or invalid values; update
the parsing logic where width, height, and scale are derived (the const width =
Number(options.width ?? 1024), height, scale lines) to validate that Number(...)
yields a finite positive number (and scale > 0), and if validation fails print a
clear error and exit (or throw) rather than proceeding with NaN/Infinity —
ensure you handle string inputs like "--width abc" by checking
Number.isFinite(value) and value > 0 and apply the default only when the option
is undefined or empty.
In `@packages/grida-canvas-sdk-render-figma/lib.ts`:
- Around line 344-469: restJsonToSceneJson currently only processes pages[0]
when rootNodeId is not provided, dropping other pages; update the multi-page
branch to iterate all pages and collect root nodes from each page, calling
iofigma.restful.factory.document for every root node across every page (use the
existing context/node_id_generator and baseGradientGen to ensure unique ids),
merge their documents into packed the same way you do for individualResults (and
derive a sensible scene.name like first non-empty page name or a default), and
aggregate imageRefsUsed from all pages; refer to restJsonToSceneJson, pages,
page, rootNodes, individualResults, and the merging logic that builds packed to
locate where to expand processing to all pages.
- Around line 139-147: applyScale currently silently no-ops for export targets
that don't have a constraints field (e.g., PDF/SVG); update applyScale to
explicitly handle non-constraints ExportAs variants instead of returning them
unchanged: inside applyScale (referencing the exportAs parameter and
types.ExportAs), add a branch that checks the exportAs format/type (e.g., "pdf"
or "svg") and either set the appropriate scale property supported by that
variant (e.g., exportAs.scale or equivalent) or throw a clear, descriptive
error/log message indicating scale cannot be applied for that format—do not
silently return the original exportAs.
---
Nitpick comments:
In `@packages/grida-canvas-sdk-render-figma/cli.ts`:
- Around line 63-101: The file-type detection is duplicated; stop re-deriving it
in runExportAll and runSingleNode by passing the isRestJson result from
resolveInput into those functions and using that flag instead of their local
isFig/isJson heuristics. Update resolveInput to return isRestJson (already
present), change the caller that invokes runExportAll and runSingleNode to
forward the returned isRestJson value, and remove or replace the internal
detection logic in runExportAll and runSingleNode so they rely solely on the
passed isRestJson flag (keep function names runExportAll and runSingleNode
unchanged to locate the edits).
- Around line 133-200: runExportAll parses .fig bytes twice: once with
iofigma.kiwi.parseFile to collect exports and again inside the renderer during
render/loadScene; to fix, propagate the already-parsed figFile into the
rendering path instead of reparsing on each render. Specifically, in
runExportAll (where figFile is created and images are extracted) attach the
parsed figFile to the FigmaDocument or include it in rendererOptions (e.g.,
rendererOptions.figFile or document.parsedFig = figFile) and update
FigmaRenderer (and its loadScene/figBytesToSceneJson usage) to check for and use
this parsed figFile before attempting to parse bytes again, so subsequent calls
to render(nid, ...) reuse the cached parse.
In `@packages/grida-canvas-sdk-render-figma/lib.ts`:
- Around line 423-426: The current code uses r.imageRefsUsed.forEach((ref:
string) => imageRefsUsed.add(ref)) which implicitly returns the Set and triggers
the lint rule; update the loop to avoid returning a value by replacing the inner
forEach with either a block arrow or a for...of loop — e.g., iterate over
r.imageRefsUsed with for (const ref of r.imageRefsUsed) {
imageRefsUsed.add(ref); } ensuring you reference the existing variables
imageRefsUsed, individualResults, and r.imageRefsUsed (and the Set.prototype.add
call) so no implicit return occurs.
- Around line 636-671: loadScene currently re-parses .fig bytes on every nodeId
by calling figBytesToSceneJson (which invokes iofigma.kiwi.parseFile); to fix,
add a cache on the instance (e.g., _figFileCache and _figImagesCache) to store
the parsed figFile and extracted images after the first figBytesToSceneJson call
and reuse them for subsequent loadScene calls for the same document payload,
falling back to calling figBytesToSceneJson only when there is no cache or when
document.payload changes; update loadScene to check the cache before calling
figBytesToSceneJson, populate the cache with figResult.sceneJson and
figResult.images (or store parsed figFile and reuse rest of conversion), and
ensure cache invalidation when the document payload or sourceType changes.
Comment on lines
+3359
to
+3391
| protected _registerImageWithCustomRid( | ||
| data: Uint8Array, | ||
| ref: string | ||
| ): Readonly<grida.program.document.ImageRef> { | ||
| assert(this._m_wasm_canvas_scene, "WASM canvas scene is not initialized"); | ||
|
|
||
| const rid = `res://images/${ref}`; | ||
| const result = this._m_wasm_canvas_scene.addImageWithId(data, rid); | ||
| if (!result) throw new Error("addImageWithId failed"); | ||
| const { width, height, type } = result; | ||
|
|
||
| const imageRef: grida.program.document.ImageRef = { | ||
| url: rid, | ||
| width, | ||
| height, | ||
| bytes: data.byteLength, | ||
| type: type as grida.program.document.ImageType, | ||
| }; | ||
|
|
||
| this.images.set(rid, imageRef); | ||
| this.doc.reduce((state) => { | ||
| state.document.images[ref] = { | ||
| url: rid, | ||
| width, | ||
| height, | ||
| bytes: data.byteLength, | ||
| type: type as grida.program.document.ImageType, | ||
| }; | ||
| return state; | ||
| }); | ||
|
|
||
| return imageRef; | ||
| } |
There was a problem hiding this comment.
Dual key scheme between this.images Map and document.images is fragile.
_registerImageWithCustomRid stores the image ref under the full RID (res://images/<ref>) in the runtime this.images Map (line 3378), but stores under the bare ref in state.document.images (line 3380). This means __is_image_registered(ref) and __get_image_ref(ref) at lines 3335–3341 only succeed when the caller passes the full URI, despite the parameter being named ref.
This dual-key contract is easy to violate — any new callsite passing just the bare ref will silently get false / null.
Consider normalizing to one convention (e.g., always use the bare ref as key in the runtime Map to match document.images, and construct the full URI only when interfacing with WASM).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor/grida-canvas/editor.ts` around lines 3359 - 3391,
_registerImageWithCustomRid currently keys this.images with the full RID while
state.document.images uses the bare ref, causing lookup mismatch; change
_registerImageWithCustomRid to keep using the constructed rid for the
ImageRef.url but use the bare ref as the Map key (call this.images.set(ref,
imageRef) instead of this.images.set(rid, ...)), ensure any other runtime
helpers that check or fetch images (__is_image_registered, __get_image_ref or
similar) continue to use the bare ref for lookups, and only construct the full
rid (`res://images/${ref}`) when calling the WASM API addImageWithId or when
serializing the ImageRef.url. Ensure the stored imageRef.url remains the full
RID but all in-memory Map keys match document.images' keys (the bare ref).
Comment on lines
+275
to
+278
| .option( | ||
| "--export-all", | ||
| "Export every node that has exportSettings (REST JSON only)" | ||
| ) |
There was a problem hiding this comment.
--export-all help text says "REST JSON only" but .fig files are also supported.
Line 277 describes --export-all as "Export every node that has exportSettings (REST JSON only)", but runExportAll (lines 147-160) has a full .fig code path that parses the file, builds a REST-like document, and collects exports. The help text should reflect this.
Proposed fix
.option(
"--export-all",
- "Export every node that has exportSettings (REST JSON only)"
+ "Export every node that has exportSettings"
)📝 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
| .option( | |
| "--export-all", | |
| "Export every node that has exportSettings (REST JSON only)" | |
| ) | |
| .option( | |
| "--export-all", | |
| "Export every node that has exportSettings" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/grida-canvas-sdk-render-figma/cli.ts` around lines 275 - 278, Update
the CLI help text for the "--export-all" option to accurately state supported
formats by removing the "(REST JSON only)" qualifier and noting that .fig files
are supported; change the description used where .option("--export-all", "Export
every node that has exportSettings (REST JSON only)") is declared so it reflects
that runExportAll handles both REST JSON and .fig file paths (see runExportAll
function for the .fig code path that parses and builds a REST-like document).
Comment on lines
+428
to
+456
| let packed: grida.program.document.IPackedSceneDocument; | ||
| if (individualResults.length === 1) { | ||
| packed = individualResults[0].document; | ||
| } else { | ||
| packed = { | ||
| bitmaps: {}, | ||
| images: {}, | ||
| nodes: {}, | ||
| links: {}, | ||
| properties: {}, | ||
| scene: { | ||
| type: "scene", | ||
| id: "main", | ||
| name: page.name ?? "Page 1", | ||
| children_refs: [], | ||
| guides: [], | ||
| edges: [], | ||
| constraints: { children: "multiple" }, | ||
| }, | ||
| }; | ||
| for (const result of individualResults) { | ||
| const d = result.document; | ||
| Object.assign(packed.nodes, d.nodes); | ||
| Object.assign(packed.links, d.links); | ||
| Object.assign(packed.images, d.images); | ||
| Object.assign(packed.bitmaps, d.bitmaps); | ||
| Object.assign(packed.properties, d.properties); | ||
| packed.scene.children_refs.push(...d.scene.children_refs); | ||
| } |
There was a problem hiding this comment.
Merging multiple root nodes via Object.assign silently drops nodes with colliding keys.
When the first page has multiple root nodes, their packed documents are merged with Object.assign(packed.nodes, d.nodes) (and similarly for links, images, etc.). If two sub-trees happen to share a generated id (unlikely with the shared counter, but possible if iofigma.restful.factory.document generates its own ids internally), later entries silently overwrite earlier ones with no warning.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/grida-canvas-sdk-render-figma/lib.ts` around lines 428 - 456, The
merge using Object.assign into packed.nodes/links/images/bitmaps/properties
while iterating individualResults can silently overwrite entries with colliding
ids; update the merge logic in the block that builds packed (the packed variable
and loop over individualResults) to detect key collisions before assigning
(e.g., iterate d.nodes keys and if packed.nodes[key] exists either throw/log
error or generate a stable unique key and remap references) and apply the same
collision-checking/remapping for links, images, bitmaps and properties so no
entries are silently dropped and scene.children_refs are updated to the remapped
ids if renaming is used.
Comment on lines
+617
to
+634
| private async ensureCanvas(width: number, height: number): Promise<Canvas> { | ||
| if (this._canvas) return this._canvas; | ||
|
|
||
| this._canvas = await createCanvas({ | ||
| backend: "raster", | ||
| width, | ||
| height, | ||
| useEmbeddedFonts: this.options.useEmbeddedFonts ?? true, | ||
| }); | ||
|
|
||
| if (this.options.loadFigmaDefaultFonts !== false) { | ||
| await ensureFigmaDefaultFonts( | ||
| this._canvas as unknown as FigmaDefaultFontsCanvas | ||
| ); | ||
| } | ||
|
|
||
| return this._canvas; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if Canvas from `@grida/canvas-wasm` exposes a resize method
echo "=== Looking for resize in canvas-wasm types ==="
rg -n 'resize' --type=ts -g '**/*canvas-wasm*' 2>/dev/null | head -20
echo -e "\n=== Searching for Canvas interface/class definition ==="
fd '.*\.(ts|tsx)$' | xargs rg -l 'interface Canvas|class Canvas|type Canvas' | head -5
echo -e "\n=== createCanvas signature and Canvas type ==="
rg -n 'createCanvas' --type=ts -C 5 2>/dev/null | head -50Repository: gridaco/grida
Length of output: 50370
🏁 Script executed:
#!/bin/bash
# Locate and read the lib.ts file to understand ensureCanvas and render behavior
echo "=== Finding lib.ts in grida-canvas-sdk-render-figma ==="
fd 'lib\.ts$' | grep 'grida-canvas-sdk-render-figma'
echo -e "\n=== Checking render method to see dimension handling ==="
fd 'lib\.ts$' | grep 'grida-canvas-sdk-render-figma' | xargs cat | grep -A 50 'async render'Repository: gridaco/grida
Length of output: 1434
🏁 Script executed:
#!/bin/bash
# Look for Canvas type definitions
echo "=== Searching canvas-wasm for Canvas type ==="
find . -path '*/canvas-wasm*' -name '*.ts' -o -name '*.d.ts' | xargs rg -l 'Canvas' 2>/dev/null | head -10
echo -e "\n=== Check if Canvas has resize, setDimensions, or similar ==="
rg -n '(resize|setWidth|setHeight|setDimensions)' --type=ts --type=d.ts 2>/dev/null | grep -i canvas | head -20Repository: gridaco/grida
Length of output: 586
🏁 Script executed:
#!/bin/bash
# Check the Canvas interface/type definition in canvas-wasm
echo "=== Canvas type definition in grida-canvas-wasm.d.ts ==="
cat crates/grida-canvas-wasm/lib/bin/grida-canvas-wasm.d.ts 2>/dev/null | head -200
echo -e "\n=== Canvas interface in api.d.ts ==="
cat crates/grida-canvas-wasm/lib/api.d.ts 2>/dev/null | head -150
echo -e "\n=== Check for resize in canvas-wasm ==="
rg -n 'resize|setSize|setDimensions' crates/grida-canvas-wasm/lib/ --type=d.ts 2>/dev/nullRepository: gridaco/grida
Length of output: 4722
🏁 Script executed:
#!/bin/bash
# Search for Canvas interface method definitions
echo "=== All methods/properties of Canvas ==="
rg -A 50 'interface Canvas|export interface Canvas|type Canvas =' crates/grida-canvas-wasm/ --type=ts --type=d.ts 2>/dev/null | head -100Repository: gridaco/grida
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Check the actual render behavior - see if it passes different dimensions multiple times
echo "=== Render calls with dimensions in the file ==="
rg -B 5 -A 5 'ensureCanvas.*width.*height' packages/grida-canvas-sdk-render-figma/lib.ts
echo -e "\n=== All render calls in test/usage ==="
fd '\.(test|spec)\.(ts|tsx)$' | xargs rg -l 'FigmaRenderer' 2>/dev/null | head -3Repository: gridaco/grida
Length of output: 883
🏁 Script executed:
#!/bin/bash
# Check createCanvas signature to understand Canvas creation API
echo "=== createCanvas function signature ==="
rg -B 5 -A 15 'createCanvas|export.*createCanvas' packages/grida-canvas-sdk-render-figma/lib.ts --type=ts | head -50
echo -e "\n=== Canvas-wasm bindings ==="
cat crates/grida-canvas-wasm/lib/modules/canvas-bindings.d.ts 2>/dev/null | head -100Repository: gridaco/grida
Length of output: 4394
🏁 Script executed:
#!/bin/bash
# Check if Canvas has any method to change dimensions
echo "=== Canvas interface methods ==="
rg 'interface.*Canvas|class.*Canvas|export.*Canvas' crates/grida-canvas-wasm/lib/ --type=d.ts -A 30 2>/dev/null | head -80Repository: gridaco/grida
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Check if Canvas class wraps _resize_surface
echo "=== Canvas wrapper class and its methods ==="
rg -B 5 -A 50 'class Canvas|export class Canvas|export interface Canvas' crates/grida-canvas-wasm/lib/ --type=ts 2>/dev/null | head -100
echo -e "\n=== createCanvas wrapper function ==="
rg -B 5 -A 30 'export.*createCanvas|function createCanvas' packages/grida-canvas-sdk-render-figma/ --type=ts 2>/dev/nullRepository: gridaco/grida
Length of output: 7650
🏁 Script executed:
#!/bin/bash
# Check the full Canvas class definition to see all its public methods
echo "=== Full Canvas class definition ==="
rg -A 150 '^export class Canvas {' crates/grida-canvas-wasm/lib/index.ts --type=ts | head -200Repository: gridaco/grida
Length of output: 218
🏁 Script executed:
#!/bin/bash
# Check if there's a resize method anywhere in Canvas
echo "=== Search for resize in Canvas class ==="
rg -B 2 -A 10 'resize|setSize|setWidth|setHeight' crates/grida-canvas-wasm/lib/index.ts --type=tsRepository: gridaco/grida
Length of output: 628
🏁 Script executed:
#!/bin/bash
# Check Scene class to understand the resize API
echo "=== Scene class with resize method ==="
rg -B 5 -A 30 'class Scene|export class Scene' crates/grida-canvas-wasm/lib/index.ts --type=ts | head -100
echo -e "\n=== Check if Canvas exposes resize publicly ==="
rg 'Canvas.*resize|Canvas.*\.resize' crates/grida-canvas-wasm/lib/index.ts --type=ts
echo -e "\n=== Full Canvas class with all methods ==="
sed -n '/^export class Canvas/,/^}/p' crates/grida-canvas-wasm/lib/index.ts | head -150Repository: gridaco/grida
Length of output: 2276
🏁 Script executed:
#!/bin/bash
# Check Scene class full definition and its resize method
echo "=== Scene class definition ==="
sed -n '/^class Scene/,/^}/p' crates/grida-canvas-wasm/lib/index.ts | head -200
echo -e "\n=== Check render method flow again ==="
sed -n '/async render/,/^ }/p' packages/grida-canvas-sdk-render-figma/lib.ts | head -40Repository: gridaco/grida
Length of output: 1119
🏁 Script executed:
#!/bin/bash
# Check test file to understand actual render usage
echo "=== Test file for FigmaRenderer ==="
head -100 packages/grida-canvas-sdk-render-figma/__tests__/refig.test.ts
echo -e "\n=== Search for multiple render calls with different sizes ==="
rg -B 3 -A 3 'render.*\{' packages/grida-canvas-sdk-render-figma/__tests__/refig.test.ts --type=ts | head -50
echo -e "\n=== Check if canvas dimensions are ever checked in the codebase ==="
rg 'canvas\.width|canvas\.height|\.width.*\.height' packages/grida-canvas-sdk-render-figma/ --type=tsRepository: gridaco/grida
Length of output: 4576
Canvas dimensions are locked to the first render() call — subsequent renders with different sizes silently use the stale viewport.
ensureCanvas() caches the canvas after the first call and ignores width/height parameters in subsequent invocations. The Canvas wrapper class does not expose a public resize method; the internal Scene's resize() is inaccessible. This means rendering multiple nodes with varying dimensions will produce incorrectly sized exports (cropped or oversized) after the initial render.
To fix: either recreate the canvas when dimensions change, or expose a resize method on the Canvas wrapper that delegates to the underlying Scene.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/grida-canvas-sdk-render-figma/lib.ts` around lines 617 - 634,
ensureCanvas currently caches this._canvas and ignores width/height on
subsequent calls, causing renders after the first to use the stale viewport;
update ensureCanvas to detect size changes and recreate the canvas (call
createCanvas again) when width or height differ from the existing canvas, or
alternatively add a public resize method on the Canvas wrapper that delegates to
the underlying Scene.resize so ensureCanvas can call that; specifically modify
ensureCanvas (and/or the Canvas wrapper class) to compare requested width/height
with the existing canvas dimensions and either recreate via createCanvas or call
Scene.resize so subsequent render() calls honor new sizes, and keep the existing
logic that still applies ensureFigmaDefaultFonts when loadFigmaDefaultFonts is
enabled.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/grida-canvas-schema/grida.ts (1)
2742-2774:⚠️ Potential issue | 🟠 MajorAdd
"path"case to the switch statement increateNodeDataFromPrototypeWithoutChildren.
createNodeDataFromPrototypeWithoutChildrendoesn't handle"path"in its switch cases (lines 2742-2774). SincePathNodePrototypeis part of theNodePrototypeunion (line 1349) andPathNode.typeis"path"(line 2453), calling this factory with a path prototype will hit the default branch and throwUnsupported node prototype type: path.Proposed fix
case "bitmap": case "ellipse": case "iframe": case "image": case "line": + case "path": case "richtext": case "tspan": case "vector": case "polygon": case "star": case "video": {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-schema/grida.ts` around lines 2742 - 2774, The switch in createNodeDataFromPrototypeWithoutChildren is missing a "path" case and thus throws for PathNodePrototype; add case "path": (alongside the other shape cases like "vector","polygon") and return the same default node object shape used in those branches—merge prototype, set name/type to prototype.type, active:true, locked:false, opacity:1, z_index:0, rotation:0, layout_target_width:100, layout_target_height:100, layout_positioning:"absolute", id:id and cast to UnknownNode; follow the same pattern as the existing cases and remove or avoid the FIXME/@ts-expect-error if not needed.
🧹 Nitpick comments (4)
packages/grida-canvas-sdk-render-figma/lib.ts (2)
638-673:.figfile is re-parsed on everyloadScenecall whennodeIdchanges.
figBytesToSceneJsoncallsiofigma.kiwi.parseFile(figBytes)each time. For multi-node export workflows (e.g.--export-all), this re-parses the entire binary for every node. Consider caching the parsedfigFileobject on the renderer instance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/lib.ts` around lines 638 - 673, The loadScene method reparses the .fig binary on every call via figBytesToSceneJson, which calls iofigma.kiwi.parseFile; to fix, parse the .fig once and cache the parsed figFile on the renderer instance (e.g., add a private field like _parsedFigFile or _figFileCache) and reuse it inside loadScene/figBytesToSceneJson; update figBytesToSceneJson (or add an overload) to accept an already-parsed figFile, change loadScene to check and populate the cache when this.document.sourceType === "fig-file", and invalidate the cached parsed file when the document payload changes so multi-node exports reuse the parsed result instead of reparsing each node.
69-99:FigmaDocumentconstructor accepts any non-null, non-array object as REST JSON.The validation at Line 92 only rejects
null, non-objects, and arrays. A plain{}or an object missingdocument.childrenpasses construction and only fails later inrestJsonToSceneJson. This is acceptable as a lightweight wrapper, but consider documenting in the JSDoc that structural validation is deferred to render time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/lib.ts` around lines 69 - 99, The FigmaDocument constructor currently treats any non-null, non-array object as a "rest-api-json" payload, which can mask missing structure until restJsonToSceneJson runs; update the JSDoc for the FigmaDocument class/constructor to explicitly state that only type checks (Uint8Array vs object) are performed here and that full structural validation (e.g., presence of document.children) is deferred to restJsonToSceneJson at render time so callers know validation is postponed; mention the relevant symbols FigmaDocument, constructor, and restJsonToSceneJson in the doc comment for clarity.packages/grida-canvas-io-figma/lib.ts (2)
1041-1121:createPathNodeFromPathduplicates ~90% ofcreateVectorNodeFromPath.Both functions share identical
base_node_trait,positioning_trait, fill/stroke/effect logic, andstrokeAsFillbranching. Only the finaltypeand data field differ (vector_networkvsdata). Consider extracting the shared property assembly into a helper to reduce the duplication surface.Sketch of shared helper
+function buildGeometryNodeBase( + parentNode: InputNode & figrest.HasGeometryTrait, + childId: string, + name: string, + options: { useFill: boolean; useStroke: boolean; strokeAsFill?: boolean } +) { + const { width, height } = getParentBounds(parentNode); + const strokeAsFill = options.strokeAsFill === true; + return { + id: childId, + ...base_node_trait({ /* same as before */ }), + ...positioning_trait({ relativeTransform: [[1,0,0],[0,1,0]], size: { x: width, y: height } }), + ...(strokeAsFill ? /* stroke-as-fill branch */ : /* normal branch */), + ...effects, + layout_target_width: width, + layout_target_height: height, + }; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-io-figma/lib.ts` around lines 1041 - 1121, createPathNodeFromPath duplicates most of createVectorNodeFromPath (base_node_trait, positioning_trait, fills_trait/stroke_trait logic, effects_trait and strokeAsFill branching); extract the shared assembly into a single helper (e.g., buildCommonPathNodeProps or assembleSharedNodeProps) that takes the common inputs (parentNode, name, childId, context, imageRefsUsed, options/useFill/useStroke/strokeAsFill, geometry bounds via getParentBounds) and returns the shared object to be spread into both createPathNodeFromPath and createVectorNodeFromPath, then have each function only add its unique fields (type + data or vector_network and layout_target_* overrides) and keep the existing try/catch and logging behavior.
1579-1581: Silentcatch {}swallows errors from volatile vectorNetwork normalization.If
normalizeRestVectorNetworkToIRsucceeds but the downstreamvn.VectorNetworkconstruction orsatisfiescheck throws, the error is silently swallowed. Aconsole.warn(matching the pattern increateVectorNodeFromPathat line 1029) would aid debugging volatile API edge cases.- } catch { - // Fall through to GroupNode + fillGeometry/strokeGeometry path - } + } catch (e) { + console.warn(`Volatile vectorNetwork normalization failed for ${node.id}:`, e); + // Fall through to GroupNode + fillGeometry/strokeGeometry path + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-io-figma/lib.ts` around lines 1579 - 1581, The empty catch around the volatile vectorNetwork normalization swallows errors from downstream vn.VectorNetwork construction or the satisfies check; update the catch in the block that calls normalizeRestVectorNetworkToIR so it logs the error (e.g., console.warn) including the error object and context (similar to the existing pattern in createVectorNodeFromPath), then fall through to the GroupNode + fillGeometry/strokeGeometry path as before. Ensure the log message names normalizeRestVectorNetworkToIR / vn.VectorNetwork / satisfies so it's easy to trace.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/packages/`@grida/refig/index.md:
- Line 5: The document's top-level heading uses "### `@grida/refig` (refig)"
which causes a heading level jump (MD001); change that heading marker from "###"
to "##" so the first visible heading is at the expected level—locate the heading
line containing "`@grida/refig` (refig)" and update the markdown heading token
accordingly.
In `@editor/grida-canvas-react-renderer-dom/nodes/node.tsx`:
- Line 228: The fillings map in node.tsx defines an entry path: "none" but the
renderer switch in the React node renderer (the switch over node.type around the
render function handling rect, ellipse, polygon, polyline, etc.) has no case
"path", so that fillings entry is dead and will cause Unknown node type: path if
used; either remove the premature fillings entry for "path" or add a matching
renderer case and implementation for "path" (mirror how "polyline" is handled:
add case "path" in the switch and call/implement the corresponding
ReactNodeRenderers.path renderer or create it if missing), updating any related
helpers so the fillings map and renderer switch remain consistent.
In `@packages/grida-canvas-sdk-render-figma/lib.ts`:
- Around line 675-711: The render() implementation currently returns the
requested viewport width/height instead of the actual exported pixel dimensions;
update it to compute and return the real exported width/height (taking node
bounds and scale into account) before building RefigRenderResult. Locate
render(), the variables width/height/scale, and the call to
canvas.exportNodeAs(normalizedNodeId, exportAs); either (A) obtain the node's
bounds via the canvas API (e.g., canvas.getNodeBounds or equivalent) and
multiply those bounds by scale to derive actualWidth/actualHeight, or (B)
modify/consume canvas.exportNodeAs to return export metadata (width/height) and
use those values; then set result.width and result.height to these computed
exported pixel dimensions instead of the original width/height. Ensure
RefigRenderResult uses actualWidth/actualHeight and keep mimeType, data, format,
and nodeId unchanged.
- Around line 424-427: The forEach callback on r.imageRefsUsed currently uses an
expression body that returns the Set (because Set.add returns the Set),
triggering the Biome lint warning; update the loop that populates imageRefsUsed
(variable imageRefsUsed and loop over individualResults and r.imageRefsUsed) to
use a block-bodied callback (e.g., r.imageRefsUsed.forEach((ref: string) => {
imageRefsUsed.add(ref); })) or replace with an explicit for..of iteration so the
callback does not return a value.
In `@packages/grida-canvas-sdk-render-figma/README.md`:
- Around line 150-161: The README's FigmaRenderer API snippet is missing the
loadFigmaDefaultFonts option defined on RefigRendererOptions; update the example
signature for FigmaRenderer/document options to include loadFigmaDefaultFonts?:
boolean (default: true) alongside useEmbeddedFonts and images so docs match the
RefigRendererOptions in lib.ts and the FAQ reference.
---
Outside diff comments:
In `@packages/grida-canvas-schema/grida.ts`:
- Around line 2742-2774: The switch in
createNodeDataFromPrototypeWithoutChildren is missing a "path" case and thus
throws for PathNodePrototype; add case "path": (alongside the other shape cases
like "vector","polygon") and return the same default node object shape used in
those branches—merge prototype, set name/type to prototype.type, active:true,
locked:false, opacity:1, z_index:0, rotation:0, layout_target_width:100,
layout_target_height:100, layout_positioning:"absolute", id:id and cast to
UnknownNode; follow the same pattern as the existing cases and remove or avoid
the FIXME/@ts-expect-error if not needed.
---
Duplicate comments:
In `@packages/grida-canvas-io-figma/lib.ts`:
- Line 3271: The Biome lint rule flags the concise arrow callback because
imageRefsUsed.add(ref) returns the Set; update the forEach callback to use a
block body so it doesn't implicitly return a value: locate the call
result.imageRefsUsed.forEach((ref: string) => imageRefsUsed.add(ref)) and change
the arrow function to a block form that calls imageRefsUsed.add(ref);
(alternatively replace with a for...of loop over result.imageRefsUsed) to
satisfy the useIterableCallbackReturn rule.
In `@packages/grida-canvas-sdk-render-figma/lib.ts`:
- Around line 619-636: ensureCanvas() currently always returns the cached
this._canvas, ignoring new width/height and locking exports to the first render;
change it to detect size mismatches and recreate the canvas when requested
width/height differ. In ensureCanvas(), compare the existing canvas dimensions
(e.g., this._canvas.width and this._canvas.height or store lastWidth/lastHeight
on the instance) against the incoming width/height, and if they differ,
dispose/replace the old canvas (call any available cleanup like
this._canvas.dispose() if supported), then call createCanvas(...) to create a
new canvas with the requested size and re-run ensureFigmaDefaultFonts when
created; keep the existing options handling (useEmbeddedFonts and
loadFigmaDefaultFonts) intact.
- Around line 344-364: The function restJsonToSceneJson currently always uses
pages[0]; change it to select the page that contains the target rootNodeId when
rootNodeId is provided: iterate over doc.document.children (pages) and for each
page inspect page.children (or descendants if necessary) to find a node with id
=== rootNodeId, then assign that page to the local page variable; fall back to
the existing pages[0] behavior only if rootNodeId is not supplied or not found.
Update references to rootNodes accordingly (still use page.children ?? []) so
conversion works for multi-page documents and matches the .fig path lookup
behavior.
- Around line 139-147: applyScale currently only handles ExportAs objects that
have a constraints field, so PDF and SVG variants silently no-op; update the
applyScale function (and its handling of types.ExportAs) to explicitly handle
PDF and SVG cases instead of returning the original object: detect when exportAs
represents a PDF or SVG and either populate the appropriate scale/ export option
field used by those types (or convert to a raster export that supports
constraints) or throw/log an explicit error indicating scale isn’t supported —
ensure you reference and modify applyScale and the types.ExportAs handling paths
so the behavior is explicit for PDF/SVG.
In `@packages/grida-canvas-sdk-render-figma/README.md`:
- Around line 308-314: The ASCII architecture diagram code block currently uses
plain triple backticks without a language; update the opening fence from ``` to
```text to satisfy markdownlint MD040 so the diagram is treated as plain text;
locate the diagram fenced block containing the "Input Conversion Rendering"
table and change its opening fence to ```text (leave the closing fence
unchanged).
---
Nitpick comments:
In `@packages/grida-canvas-io-figma/lib.ts`:
- Around line 1041-1121: createPathNodeFromPath duplicates most of
createVectorNodeFromPath (base_node_trait, positioning_trait,
fills_trait/stroke_trait logic, effects_trait and strokeAsFill branching);
extract the shared assembly into a single helper (e.g., buildCommonPathNodeProps
or assembleSharedNodeProps) that takes the common inputs (parentNode, name,
childId, context, imageRefsUsed, options/useFill/useStroke/strokeAsFill,
geometry bounds via getParentBounds) and returns the shared object to be spread
into both createPathNodeFromPath and createVectorNodeFromPath, then have each
function only add its unique fields (type + data or vector_network and
layout_target_* overrides) and keep the existing try/catch and logging behavior.
- Around line 1579-1581: The empty catch around the volatile vectorNetwork
normalization swallows errors from downstream vn.VectorNetwork construction or
the satisfies check; update the catch in the block that calls
normalizeRestVectorNetworkToIR so it logs the error (e.g., console.warn)
including the error object and context (similar to the existing pattern in
createVectorNodeFromPath), then fall through to the GroupNode +
fillGeometry/strokeGeometry path as before. Ensure the log message names
normalizeRestVectorNetworkToIR / vn.VectorNetwork / satisfies so it's easy to
trace.
In `@packages/grida-canvas-sdk-render-figma/lib.ts`:
- Around line 638-673: The loadScene method reparses the .fig binary on every
call via figBytesToSceneJson, which calls iofigma.kiwi.parseFile; to fix, parse
the .fig once and cache the parsed figFile on the renderer instance (e.g., add a
private field like _parsedFigFile or _figFileCache) and reuse it inside
loadScene/figBytesToSceneJson; update figBytesToSceneJson (or add an overload)
to accept an already-parsed figFile, change loadScene to check and populate the
cache when this.document.sourceType === "fig-file", and invalidate the cached
parsed file when the document payload changes so multi-node exports reuse the
parsed result instead of reparsing each node.
- Around line 69-99: The FigmaDocument constructor currently treats any
non-null, non-array object as a "rest-api-json" payload, which can mask missing
structure until restJsonToSceneJson runs; update the JSDoc for the FigmaDocument
class/constructor to explicitly state that only type checks (Uint8Array vs
object) are performed here and that full structural validation (e.g., presence
of document.children) is deferred to restJsonToSceneJson at render time so
callers know validation is postponed; mention the relevant symbols
FigmaDocument, constructor, and restJsonToSceneJson in the doc comment for
clarity.
| title: refig | ||
| --- | ||
|
|
||
| ### `@grida/refig` (refig) |
There was a problem hiding this comment.
Fix heading level jump (MD001).
Line 5 starts with ### right after frontmatter. Markdownlint expects the first heading to be ## (or # depending on the style). Adjusting to ## will satisfy the rule.
🔧 Proposed fix
-### `@grida/refig` (refig)
+## `@grida/refig` (refig)📝 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
| ### `@grida/refig` (refig) | |
| ## `@grida/refig` (refig) |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/packages/`@grida/refig/index.md at line 5, The document's top-level
heading uses "### `@grida/refig` (refig)" which causes a heading level jump
(MD001); change that heading marker from "###" to "##" so the first visible
heading is at the expected level—locate the heading line containing
"`@grida/refig` (refig)" and update the markdown heading token accordingly.
| instance: "none", | ||
| line: "none", | ||
| vector: "none", | ||
| path: "none", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and its size
wc -l editor/grida-canvas-react-renderer-dom/nodes/node.tsxRepository: gridaco/grida
Length of output: 114
🏁 Script executed:
# Read the critical sections mentioned in the review
# Lines 70-104 (renderer switch)
echo "=== Lines 70-104 (renderer switch) ===" && \
sed -n '70,104p' editor/grida-canvas-react-renderer-dom/nodes/node.tsx
echo -e "\n=== Lines 220-235 (fillings map area around line 228) ===" && \
sed -n '220,235p' editor/grida-canvas-react-renderer-dom/nodes/node.tsx
echo -e "\n=== Lines 180-195 (fillings usage) ===" && \
sed -n '180,195p' editor/grida-canvas-react-renderer-dom/nodes/node.tsxRepository: gridaco/grida
Length of output: 2122
🏁 Script executed:
# Search for ReactNodeRenderers definition to check if it has a "path" entry
echo "=== Searching for ReactNodeRenderers definition ===" && \
rg -n "ReactNodeRenderers" --type=ts --type=tsx -A 20 editor/grida-canvas-react-renderer-dom/Repository: gridaco/grida
Length of output: 153
🏁 Script executed:
# Search for PathNode JSDoc to verify the "Not yet supported" claim
echo "=== Searching for PathNode JSDoc ===" && \
rg -n "PathNode" --type=ts --type=tsx -B 5 -A 5 editor/grida-canvas-react-renderer-dom/Repository: gridaco/grida
Length of output: 138
🏁 Script executed:
# Let's try a simpler search for ReactNodeRenderers without the problematic flag
echo "=== Searching for ReactNodeRenderers definition ===" && \
rg -n "ReactNodeRenderers" --type=ts -A 20 editor/grida-canvas-react-renderer-dom/Repository: gridaco/grida
Length of output: 5544
🏁 Script executed:
# Search for PathNode and polyline references
echo "=== Searching for PathNode ===" && \
rg -n "PathNode" --type=ts -B 3 -A 3 editor/grida-canvas-react-renderer-dom/ && \
echo -e "\n=== Searching for polyline ===" && \
rg -n "polyline" --type=ts -B 2 -A 2 editor/grida-canvas-react-renderer-dom/Repository: gridaco/grida
Length of output: 87
🏁 Script executed:
# Let's also check what node types are actually used/expected in the codebase
echo "=== Checking all node types referenced in fillings ===" && \
sed -n '200,235p' editor/grida-canvas-react-renderer-dom/nodes/node.tsxRepository: gridaco/grida
Length of output: 839
path added to fillings but not handled in the renderer switch — unreachable dead code.
The fillings map includes path: "none" (line 228), but the renderer switch at lines 70–104 has no case "path". If a path node attempts to render, it will fall through to the default branch and throw Unknown node type: path at runtime. The fillings entry cannot be reached.
Note that polyline is properly supported with both a renderer case (line 91) and a ReactNodeRenderers.polyline entry. For consistency, either add a "path" case alongside the other shape types once a renderer exists, or remove the premature fillings entry.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor/grida-canvas-react-renderer-dom/nodes/node.tsx` at line 228, The
fillings map in node.tsx defines an entry path: "none" but the renderer switch
in the React node renderer (the switch over node.type around the render function
handling rect, ellipse, polygon, polyline, etc.) has no case "path", so that
fillings entry is dead and will cause Unknown node type: path if used; either
remove the premature fillings entry for "path" or add a matching renderer case
and implementation for "path" (mirror how "polyline" is handled: add case "path"
in the switch and call/implement the corresponding ReactNodeRenderers.path
renderer or create it if missing), updating any related helpers so the fillings
map and renderer switch remain consistent.
Comment on lines
+424
to
+427
| const imageRefsUsed = new Set<string>(); | ||
| for (const r of individualResults) { | ||
| r.imageRefsUsed.forEach((ref: string) => imageRefsUsed.add(ref)); | ||
| } |
There was a problem hiding this comment.
Static analysis: forEach callback should not return a value.
Set.add() returns the Set, so the .forEach callback implicitly returns it. Use a block body to suppress the Biome lint warning.
Proposed fix
- r.imageRefsUsed.forEach((ref: string) => imageRefsUsed.add(ref));
+ r.imageRefsUsed.forEach((ref: string) => { imageRefsUsed.add(ref); });🧰 Tools
🪛 Biome (2.3.14)
[error] 426-426: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/grida-canvas-sdk-render-figma/lib.ts` around lines 424 - 427, The
forEach callback on r.imageRefsUsed currently uses an expression body that
returns the Set (because Set.add returns the Set), triggering the Biome lint
warning; update the loop that populates imageRefsUsed (variable imageRefsUsed
and loop over individualResults and r.imageRefsUsed) to use a block-bodied
callback (e.g., r.imageRefsUsed.forEach((ref: string) => {
imageRefsUsed.add(ref); })) or replace with an explicit for..of iteration so the
callback does not return a value.
Comment on lines
+675
to
+711
| async render( | ||
| nodeId: string, | ||
| renderOptions: RefigRenderOptions | ||
| ): Promise<RefigRenderResult> { | ||
| const normalizedNodeId = nodeId.trim(); | ||
| if (!normalizedNodeId) { | ||
| throw new Error("FigmaRenderer.render: nodeId is required"); | ||
| } | ||
|
|
||
| const width = renderOptions.width ?? 1024; | ||
| const height = renderOptions.height ?? 1024; | ||
| const scale = renderOptions.scale ?? 1; | ||
| const format = renderOptions.format; | ||
| const mimeType = resolveMimeType(format); | ||
|
|
||
| const canvas = await this.ensureCanvas(width, height); | ||
| if (this._requestedNodeId !== normalizedNodeId) { | ||
| this._sceneLoaded = false; | ||
| } | ||
| this.loadScene(canvas, normalizedNodeId); | ||
|
|
||
| let exportAs = mapFormat(format); | ||
| if (scale !== 1) { | ||
| exportAs = applyScale(exportAs, scale); | ||
| } | ||
|
|
||
| const { data } = canvas.exportNodeAs(normalizedNodeId, exportAs); | ||
|
|
||
| return { | ||
| data, | ||
| format, | ||
| mimeType, | ||
| nodeId: normalizedNodeId, | ||
| width, | ||
| height, | ||
| }; | ||
| } |
There was a problem hiding this comment.
render() returns the requested width/height, not the actual export dimensions.
Lines 708–709 return the width and height from renderOptions (or the 1024 default), but the real pixel dimensions of the exported image depend on the node bounds and scale factor. For instance, rendering at scale: 2 would produce a 2× image, yet result.width still reports the unscaled viewport size. This can mislead callers relying on RefigRenderResult for layout or metadata.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/grida-canvas-sdk-render-figma/lib.ts` around lines 675 - 711, The
render() implementation currently returns the requested viewport width/height
instead of the actual exported pixel dimensions; update it to compute and return
the real exported width/height (taking node bounds and scale into account)
before building RefigRenderResult. Locate render(), the variables
width/height/scale, and the call to canvas.exportNodeAs(normalizedNodeId,
exportAs); either (A) obtain the node's bounds via the canvas API (e.g.,
canvas.getNodeBounds or equivalent) and multiply those bounds by scale to derive
actualWidth/actualHeight, or (B) modify/consume canvas.exportNodeAs to return
export metadata (width/height) and use those values; then set result.width and
result.height to these computed exported pixel dimensions instead of the
original width/height. Ensure RefigRenderResult uses actualWidth/actualHeight
and keep mimeType, data, format, and nodeId unchanged.
Comment on lines
+150
to
+161
| ```ts | ||
| const renderer = new FigmaRenderer(document: FigmaDocument, options?: { | ||
| useEmbeddedFonts?: boolean; // default: true | ||
| images?: Record<string, Uint8Array>; // image ref → bytes; used for REST API IMAGE fills | ||
| }); | ||
|
|
||
| writeFileSync("out.png", data); | ||
| const result = await renderer.render(nodeId: string, { | ||
| format: "png" | "jpeg" | "webp" | "pdf" | "svg"; | ||
| width?: number; // default: 1024 | ||
| height?: number; // default: 1024 | ||
| scale?: number; // default: 1 | ||
| }); |
There was a problem hiding this comment.
FigmaRenderer API docs omit the loadFigmaDefaultFonts option.
The RefigRendererOptions interface in lib.ts defines loadFigmaDefaultFonts?: boolean (default true), and the FAQ section at Line 367 references it. However, the API reference here only lists useEmbeddedFonts and images. Add loadFigmaDefaultFonts to keep the docs consistent.
Proposed fix
const renderer = new FigmaRenderer(document: FigmaDocument, options?: {
useEmbeddedFonts?: boolean; // default: true
+ loadFigmaDefaultFonts?: boolean; // default: true — load Figma default fonts from CDN
images?: Record<string, Uint8Array>; // image ref → bytes; used for REST API IMAGE fills
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/grida-canvas-sdk-render-figma/README.md` around lines 150 - 161, The
README's FigmaRenderer API snippet is missing the loadFigmaDefaultFonts option
defined on RefigRendererOptions; update the example signature for
FigmaRenderer/document options to include loadFigmaDefaultFonts?: boolean
(default: true) alongside useEmbeddedFonts and images so docs match the
RefigRendererOptions in lib.ts and the FAQ reference.
- Modified the CLI to allow omitting the --out option, defaulting to the OS temp directory when using --export-all or both --format and --node. - Improved error handling for output directory validation. - Updated README to reflect changes in usage and examples for better clarity.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/grida-canvas-sdk-render-figma/cli.ts (2)
308-334: IIFE foroutDirresolution works but is hard to scan.The inline IIFE (lines 312–327) for resolving and validating the output directory is functionally correct but dense. Consider extracting it into a small helper for readability. Not blocking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/cli.ts` around lines 308 - 334, The inline IIFE that computes outDir when exportAll is true is hard to scan; extract that logic into a small helper function (e.g., resolveOutDir(outPath): string) that accepts outPath, performs path.resolve, checks existsSync/statSync to ensure it's a directory or calls program.error with the same message, creates the directory with mkdirSync({ recursive: true }) if missing, and returns mkdtempSync(path.join(tmpdir(), "refig-export-")) when outPath is undefined; then replace the IIFE with a call to resolveOutDir(outPath) before calling runExportAll(documentPath, outDir, imagesDir, options.skipDefaultFonts === true).
65-103:statSyncon a nonexistent path gives a crypticENOENTerror.If the user passes a path that doesn't exist,
statSyncat Line 75 throws a raw Node.js error. A quickexistsSynccheck beforehand would give a friendlier message.Proposed fix
function resolveInput( inputPath: string, explicitImagesDir: string | undefined ): { ... } { const resolved = path.resolve(inputPath); + if (!existsSync(resolved)) { + throw new Error(`Input path does not exist: ${resolved}`); + } const stat = statSync(resolved);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/cli.ts` around lines 65 - 103, The resolveInput function currently calls statSync(resolved) without checking existence, causing a raw ENOENT on missing paths; add a pre-check using existsSync(resolved) and if false throw a clear Error (e.g., "Input path not found: <resolved>") before calling statSync. Keep the rest of resolveInput logic intact (directory vs file handling, explicitImagesDir resolution, and isRestJson detection) and reference the resolved variable and explicitImagesDir when composing the friendly error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/grida-canvas-sdk-render-figma/cli.ts`:
- Around line 277-280: Update the CLI help text for the --export-all option to
reflect that .fig is supported: change the description string currently "Export
every node that has exportSettings (REST JSON only)" to mention .fig support
(e.g., remove "REST JSON only" or add ".fig supported"); locate the option
declaration for "--export-all" in the CLI setup and synchronize its description
with the implementation in runExportAll so users see accurate help text.
- Around line 341-343: After parsing options into const width =
Number(options.width ?? 1024), height = Number(options.height ?? 1024), and
scale = Number(options.scale ?? 1) add validation guards that check
Number.isFinite(width), Number.isFinite(height) and Number.isFinite(scale) (or
!Number.isNaN) and handle invalid values: throw or exit with a clear error
message mentioning the flag name (e.g., "--width must be a number") or provide a
safe default; update the code around these variables (in cli.ts where
width/height/scale are declared) to perform this check immediately and stop
execution or normalize values before passing them to the renderer.
In `@packages/grida-canvas-sdk-render-figma/README.md`:
- Around line 277-280: Update the CLI help text for the --export-all option to
accurately reflect supported formats (include .fig in addition to "REST JSON
only"); locate the CLI flag description for --export-all (referenced alongside
the runExportAll handler in cli.ts) and change the string shown in the
help/usage output so it mentions both REST JSON and .fig export support to match
the behavior in runExportAll and the README.
- Around line 315-321: The fenced code block containing the ASCII architecture
diagram is missing a language identifier; update the triple-backtick opening to
include a plain text specifier (e.g., change ``` to ```text) so markdownlint
MD040 is satisfied and the block is treated as text; locate the diagram block
beginning with "Input Conversion
Rendering" and add the language token to its opening fence.
- Around line 150-154: The README's FigmaRenderer constructor signature omits
the loadFigmaDefaultFonts option; update the API docs to include
loadFigmaDefaultFonts?: boolean in the options list (alongside useEmbeddedFonts
and images) to reflect RefigRendererOptions in lib.ts and match the FAQ. Locate
references to FigmaRenderer and RefigRendererOptions and add the
loadFigmaDefaultFonts entry with a short note about its default behavior.
---
Nitpick comments:
In `@packages/grida-canvas-sdk-render-figma/cli.ts`:
- Around line 308-334: The inline IIFE that computes outDir when exportAll is
true is hard to scan; extract that logic into a small helper function (e.g.,
resolveOutDir(outPath): string) that accepts outPath, performs path.resolve,
checks existsSync/statSync to ensure it's a directory or calls program.error
with the same message, creates the directory with mkdirSync({ recursive: true })
if missing, and returns mkdtempSync(path.join(tmpdir(), "refig-export-")) when
outPath is undefined; then replace the IIFE with a call to
resolveOutDir(outPath) before calling runExportAll(documentPath, outDir,
imagesDir, options.skipDefaultFonts === true).
- Around line 65-103: The resolveInput function currently calls
statSync(resolved) without checking existence, causing a raw ENOENT on missing
paths; add a pre-check using existsSync(resolved) and if false throw a clear
Error (e.g., "Input path not found: <resolved>") before calling statSync. Keep
the rest of resolveInput logic intact (directory vs file handling,
explicitImagesDir resolution, and isRestJson detection) and reference the
resolved variable and explicitImagesDir when composing the friendly error.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
day-319-refig.mp4
refig - render figma files (without figma)
ridsupport for rendererres://images/external-image-with-own-uri.pngFigma exports are easy—until exporting becomes infrastructure.
Teams start with “click Export” and end up needing thumbnails, asset pipelines, visual diffs, and repeatable builds. That’s where the usual options start to hurt:
refig is built for that gap.
What is refig?
@grida/refig(“render figma”) is a headless Figma renderer. It turns a Figma document + node id into PNG, JPEG, WebP, PDF, or SVG—without opening Figma and without driving a browser UI.It works with:
.figfiles (offline, reproducible)GET /v1/files/:key) when you already have your own ingestion layerYou can use it as a CLI (
refig) or as a library in Node.js and the browser.@grida/refigon npmThe idea: deterministic rendering you can build on
When rendering is deterministic and programmable, a bunch of workflows become straightforward:
.figsnapshots and reproduce outputs later—no tokens, no network.In other words: refig is less about “export an image” and more about making Figma rendering a reliable building block.
What refig intentionally doesn’t do
refig is opinionated about scope so it stays composable:
A tiny taste (CLI)
If you just want to feel what it’s like:
From there, most teams graduate quickly to: “render N nodes”, “render on every commit”, or “render on demand”.
Where this is headed
We’re treating refig as a foundation for “design → build” automation:
If that sounds like your problem, start with the docs (high-level) and the npm page (full API/CLI reference), then wire it into the pipeline you already have:
@grida/refigdocs@grida/refigon npmSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores