fix: pdf export #8564
fix: pdf export #8564
Conversation
📝 WalkthroughWalkthroughAdds a complete PDF export feature: HTTP controller and schema, TipTap-to-PDF rendering stack (colors, styles, icons, node/mark renderers), export service orchestrating content fetch/image processing/rendering, page-service helpers, tests, and related package/config updates. Changes
Sequence DiagramsequenceDiagram
participant Client
participant PdfExportController
participant PdfExportService
participant PageService
participant ImageProcessor
participant PdfRenderer
Client->>PdfExportController: POST /export-pdf (with cookie)
activate PdfExportController
PdfExportController->>PdfExportController: parseRequest (validate, auth)
alt Auth Failed
PdfExportController-->>Client: 401 Unauthorized
else Validation Failed
PdfExportController-->>Client: 400 Bad Request
else Success
PdfExportController->>PdfExportService: exportToPdf(input)
activate PdfExportService
PdfExportService->>PageService: fetchPageContent(pageId)
activate PageService
PageService-->>PdfExportService: contentJSON, titleHTML, descriptionBinary
deactivate PageService
PdfExportService->>PdfExportService: extractImageAssetIds(content)
PdfExportService->>PageService: fetchUserMentions(pageId)
PageService-->>PdfExportService: userMentions[]
alt Images to process
PdfExportService->>PageService: resolveImageAssetUrls(assetIds)
PageService-->>PdfExportService: assetId→URL map
loop Each Image
PdfExportService->>ImageProcessor: fetch & process (sharp) → base64
ImageProcessor-->>PdfExportService: data URL
end
end
PdfExportService->>PdfRenderer: renderPlaneDocToPdfBuffer(document, metadata)
activate PdfRenderer
PdfRenderer-->>PdfExportService: PDF Buffer
deactivate PdfRenderer
PdfExportService-->>PdfExportController: { pdfBuffer, outputFileName, pageId }
deactivate PdfExportService
PdfExportController->>Client: 200 + PDF stream (Content-Disposition)
end
deactivate PdfExportController
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@apps/live/package.json`:
- Around line 31-33: Add the listed external packages into the pnpm workspace
catalog with the exact versions provided (entries for "@effect/platform",
"@effect/platform-node", "@fontsource/inter", "@react-pdf/renderer",
"@react-pdf/types", "effect", "sharp", "@types/pdf-parse",
"@vitest/coverage-v8", "pdf-parse", and "vitest" using the versions in the
review), then update apps/live/package.json dependency entries for those same
package names to reference the catalog: versions instead of direct semver
ranges; ensure you modify the pnpm-workspace.yaml catalog section to include
each package/version and replace the corresponding dependency strings in
package.json with catalog:package@version tokens.
In `@apps/live/src/lib/pdf/plane-pdf-exporter.tsx`:
- Around line 14-45: Wrap the Font.register(...) invocation in a try-catch to
surface missing or incompatible font files: catch any error thrown by
Font.register (referencing Font.register and interFontDir / the hardcoded
filenames like inter-latin-400-normal.woff2) log a clear diagnostic (e.g.,
console.error or processLogger.error) that includes the error object and the
attempted file paths, then rethrow the error so callers know registration
failed; this ensures failures in plane-pdf-exporter.tsx are logged and do not
fail silently.
In `@apps/live/src/services/page/core.service.ts`:
- Around line 41-62: fetchDescriptionBinary currently assumes axios returns a
Node Buffer and throws if Buffer.isBuffer fails; instead detect and convert
ArrayBuffer or typed array views to a Buffer before returning. In the
fetchDescriptionBinary method (responseType: "arraybuffer"), check if
response.data is an ArrayBuffer or ArrayBuffer view (use ArrayBuffer.isView) and
create a Buffer from it (Buffer.from(...)), then proceed with the
Buffer.isBuffer check/return; keep the existing error handling (AppError and
logger) intact.
In `@apps/live/vitest.config.ts`:
- Around line 1-21: The config uses __dirname in the resolve.alias
(path.resolve(__dirname, "./src")) which breaks under ESM; replace this by
deriving dirname from import.meta.url (use fileURLToPath(import.meta.url) to get
the file path then path.dirname) and pass that dirname into path.resolve for the
"@" alias; add the import for fileURLToPath from "url" and compute a const
dirname (or similar) at the top of the module, then update the alias to use
path.resolve(dirname, "./src").
🧹 Nitpick comments (6)
apps/live/src/controllers/pdf-export.controller.ts (2)
15-27: Cookie validation has a subtle correctness issue.The cookie variable is assigned
req.headers.cookie || "", which means it will be an empty string when the cookie header is absent. However,!cookieevaluates totruefor an empty string, so the logic works correctly. That said, the pattern is slightly confusing—consider simplifying:- const cookie = req.headers.cookie || ""; - if (!cookie) { + const cookie = req.headers.cookie; + if (!cookie) {This makes the intent clearer and avoids the unnecessary fallback to empty string before immediately checking for absence.
113-118: Type narrowing relies on structural discrimination.The check
if ("error" in result && "status" in result)works becausePdfExportResultcontainspdfBuffer,outputFileName, andpageId—none of which overlap witherror/status. However, this is fragile ifPdfExportResultever gains anerrororstatusfield.Consider using a discriminated union pattern with a
_tagorsuccessfield for more robust type narrowing.apps/live/tests/lib/pdf/pdf-rendering.test.ts (1)
442-443: Buffer length comparison may produce flaky tests.Asserting that
a4Buffer.length !== letterBuffer.lengthrelies on PDF generation producing different byte counts for different page sizes. While generally true, this could be flaky if:
- Compression algorithms produce varying output
- Future optimizations change the generation
Consider verifying page dimensions from PDF metadata instead if the test framework supports it.
Also applies to: 464-465
apps/live/src/lib/pdf/plane-pdf-exporter.tsx (1)
67-82: Buffer/Blob conversion is straightforward but could benefit from error handling.Both
renderPlaneDocToPdfBufferandrenderPlaneDocToPdfBlobdon't have try-catch blocks. If PDF generation fails (e.g., invalid image URLs, unsupported content), the error will propagate up. While this may be intentional for the caller to handle, consider whether wrapping in a domain-specific error would improve debugging.apps/live/src/services/pdf-export/pdf-export.service.ts (1)
23-27: DuplicateTipTapNodetype definition.This local
TipTapNodetype duplicates the definition in@/lib/pdf/types.ts. Import it instead to maintain a single source of truth and avoid drift.Proposed fix
-import type { PDFExportMetadata, TipTapDocument } from "@/lib/pdf"; +import type { PDFExportMetadata, TipTapDocument, TipTapNode } from "@/lib/pdf"; import { renderPlaneDocToPdfBuffer } from "@/lib/pdf";Then remove lines 23-27.
apps/live/src/lib/pdf/colors.ts (1)
86-93: Consider handling edge cases in CSS variable parsing.The regex handles the expected pattern well. Consider whether malformed inputs (e.g., unclosed parentheses, extra whitespace) should return
nullsilently or warrant a warning log for debugging.
| "@effect/platform": "^0.94.0", | ||
| "@effect/platform-node": "^0.104.0", | ||
| "@fontsource/inter": "5.2.8", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the structure and locate the relevant files
git ls-files | grep -E "(apps/live/package.json|pnpm-workspace.yaml)" | head -20Repository: makeplane/plane
Length of output: 101
🏁 Script executed:
# Read the apps/live/package.json file to see the actual dependencies at the mentioned lines
cat -n apps/live/package.jsonRepository: makeplane/plane
Length of output: 3319
🏁 Script executed:
# Also check the pnpm-workspace.yaml to see if catalog is defined
cat pnpm-workspace.yamlRepository: makeplane/plane
Length of output: 994
Add packages to catalog: in both files.
Lines 31–33, 43–45, 52, 58, and 73–80 introduce external packages with version ranges. Per repo policy, external dependencies must use catalog: with versions declared in pnpm-workspace.yaml. These packages are missing from the catalog and need to be added there before updating package.json references.
Update pnpm-workspace.yaml to include:
"@effect/platform": ^0.94.0
"@effect/platform-node": ^0.104.0
"@fontsource/inter": 5.2.8
"@react-pdf/renderer": ^4.3.0
"@react-pdf/types": ^2.9.2
"effect": ^3.16.3
"sharp": ^0.34.3
"@types/pdf-parse": ^1.1.5
"@vitest/coverage-v8": ^4.0.8
"pdf-parse": ^2.4.5
"vitest": ^4.0.8Then update apps/live/package.json to use catalog: for these entries, as shown in the original diff.
🤖 Prompt for AI Agents
In `@apps/live/package.json` around lines 31 - 33, Add the listed external
packages into the pnpm workspace catalog with the exact versions provided
(entries for "@effect/platform", "@effect/platform-node", "@fontsource/inter",
"@react-pdf/renderer", "@react-pdf/types", "effect", "sharp",
"@types/pdf-parse", "@vitest/coverage-v8", "pdf-parse", and "vitest" using the
versions in the review), then update apps/live/package.json dependency entries
for those same package names to reference the catalog: versions instead of
direct semver ranges; ensure you modify the pnpm-workspace.yaml catalog section
to include each package/version and replace the corresponding dependency strings
in package.json with catalog:package@version tokens.
| Font.register({ | ||
| family: "Inter", | ||
| fonts: [ | ||
| { | ||
| src: path.join(interFontDir, "files/inter-latin-400-normal.woff2"), | ||
| fontWeight: 400, | ||
| }, | ||
| { | ||
| src: path.join(interFontDir, "files/inter-latin-400-italic.woff2"), | ||
| fontWeight: 400, | ||
| fontStyle: "italic", | ||
| }, | ||
| { | ||
| src: path.join(interFontDir, "files/inter-latin-600-normal.woff2"), | ||
| fontWeight: 600, | ||
| }, | ||
| { | ||
| src: path.join(interFontDir, "files/inter-latin-600-italic.woff2"), | ||
| fontWeight: 600, | ||
| fontStyle: "italic", | ||
| }, | ||
| { | ||
| src: path.join(interFontDir, "files/inter-latin-700-normal.woff2"), | ||
| fontWeight: 700, | ||
| }, | ||
| { | ||
| src: path.join(interFontDir, "files/inter-latin-700-italic.woff2"), | ||
| fontWeight: 700, | ||
| fontStyle: "italic", | ||
| }, | ||
| ], | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the file
find . -name "plane-pdf-exporter.tsx" -type fRepository: makeplane/plane
Length of output: 105
🏁 Script executed:
# Check the file content around the font registration
cat -n apps/live/src/lib/pdf/plane-pdf-exporter.tsx | head -50Repository: makeplane/plane
Length of output: 2048
🏁 Script executed:
# Check if `@fontsource/inter` is in package.json
find . -name "package.json" -type f | xargs grep -l "@fontsource/inter"Repository: makeplane/plane
Length of output: 83
🏁 Script executed:
# Check the actual structure of `@fontsource/inter` if it exists in node_modules
find . -path "*/node_modules/@fontsource/inter/files/*" -type f 2>/dev/null | head -20Repository: makeplane/plane
Length of output: 41
🏁 Script executed:
# Check which version of `@fontsource/inter` is in the package.json
cat apps/live/package.json | grep -A 2 -B 2 "@fontsource/inter"Repository: makeplane/plane
Length of output: 262
🏁 Script executed:
# Check if there's any error handling around Font.register in the file
cat -n apps/live/src/lib/pdf/plane-pdf-exporter.tsx | grep -A 5 -B 5 "Font.register"Repository: makeplane/plane
Length of output: 484
🏁 Script executed:
# Search for any error handling or try-catch blocks in this file
cat apps/live/src/lib/pdf/plane-pdf-exporter.tsx | grep -E "(try|catch|Error|error)" -iRepository: makeplane/plane
Length of output: 41
🌐 Web query:
@fontsource/inter package file structure woff2 filenames
💡 Result:
Short answer — filenames use this pattern inside the package/files folder:
- inter---<style>.woff2
- subset ∈ {latin, latin-ext, cyrillic, cyrillic-ext, greek, greek-ext, vietnamese, …}
- weight ∈ {100,200,300,400,500,600,700,800,900}
- style ∈ {normal, italic}
- example: inter-cyrillic-400-normal.woff2. [1][2]
You’ll also see “all” bundles named:
- inter-all--normal.woff (and .woff2 in some releases)
- example: inter-all-400-normal.woff. [1]
Sources: package file listing on UNPKG and the @fontsource/inter npm/package pages. [1][2]
References
- [1] UNPKG file listing for
@fontsource/inter(shows inter--.woff2 filenames). - [2]
@fontsource/internpm / Fontsource docs (package structure and supported subsets/weights).
Add error handling for font registration to handle missing or incompatible @fontsource/inter package files.
The Font.register() call on line 14 lacks error handling. While the font directory is dynamically resolved via require.resolve(), the specific font filenames are hardcoded (e.g., inter-latin-400-normal.woff2). If these files are missing, renamed, or the package structure changes, the registration will fail silently or throw at runtime without diagnostic information. Add a try-catch block with appropriate error logging to aid debugging:
try {
Font.register({
family: "Inter",
fonts: [
// ... font definitions
],
});
} catch (error) {
console.error("Failed to register Inter font:", error);
throw error;
}This aligns with the coding guideline to "use try-catch with proper error types and log errors appropriately."
🤖 Prompt for AI Agents
In `@apps/live/src/lib/pdf/plane-pdf-exporter.tsx` around lines 14 - 45, Wrap the
Font.register(...) invocation in a try-catch to surface missing or incompatible
font files: catch any error thrown by Font.register (referencing Font.register
and interFontDir / the hardcoded filenames like inter-latin-400-normal.woff2)
log a clear diagnostic (e.g., console.error or processLogger.error) that
includes the error object and the attempted file paths, then rethrow the error
so callers know registration failed; this ensures failures in
plane-pdf-exporter.tsx are logged and do not fail silently.
| async fetchDescriptionBinary(pageId: string): Promise<Buffer> { | ||
| try { | ||
| const response = await this.get(`${this.basePath}/pages/${pageId}/description/`, { | ||
| headers: { | ||
| ...this.getHeader(), | ||
| "Content-Type": "application/octet-stream", | ||
| }, | ||
| responseType: "arraybuffer", | ||
| }); | ||
| const data = response?.data; | ||
| if (!Buffer.isBuffer(data)) { | ||
| throw new Error("Expected response to be a Buffer"); | ||
| } | ||
| return data; | ||
| } catch (error) { | ||
| const appError = new AppError(error, { | ||
| context: { operation: "fetchDescriptionBinary", pageId }, | ||
| }); | ||
| logger.error("Failed to fetch page description binary", appError); | ||
| throw appError; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if api.service.ts exists and get its size
wc -l apps/live/src/services/api.service.ts 2>/dev/null || echo "File not found, searching for it"
# If not found, search for api service files
fd "api.*service" apps/live/src/services/ 2>/dev/null | head -20Repository: makeplane/plane
Length of output: 137
🏁 Script executed:
cat -n apps/live/src/services/api.service.tsRepository: makeplane/plane
Length of output: 2047
🏁 Script executed:
# Check the page/core.service.ts file to see the exact implementation
sed -n '41,62p' apps/live/src/services/page/core.service.ts
# Also search for any other uses of arraybuffer in the codebase to see how they handle it
rg -n "arraybuffer" apps/live/src/Repository: makeplane/plane
Length of output: 876
🏁 Script executed:
# Search for other buffer/arraybuffer handling patterns
rg -n "Buffer.isBuffer|response\.data|arraybuffer" apps/live/src/services/ -A 2 -B 2
# Check if there are other fetchDescriptionBinary calls or similar patterns
rg -n "Promise<Buffer>|Promise<ArrayBuffer>" apps/live/src/Repository: makeplane/plane
Length of output: 871
Handle ArrayBuffer/TypedArray responses before Buffer checks.
When axios is configured with responseType: "arraybuffer", it may return ArrayBuffer or typed array views depending on the environment. Line 51's Buffer.isBuffer(data) check will fail for these types, causing the method to always throw despite receiving valid data. Convert non-Buffer responses to Buffer before returning.
🛠️ Proposed fix
- const data = response?.data;
- if (!Buffer.isBuffer(data)) {
- throw new Error("Expected response to be a Buffer");
- }
- return data;
+ const data = response?.data;
+ if (Buffer.isBuffer(data)) {
+ return data;
+ }
+ if (data instanceof ArrayBuffer) {
+ return Buffer.from(data);
+ }
+ if (ArrayBuffer.isView(data)) {
+ return Buffer.from(data.buffer, data.byteOffset, data.byteLength);
+ }
+ throw new Error("Expected response to be a Buffer or ArrayBuffer");📝 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.
| async fetchDescriptionBinary(pageId: string): Promise<Buffer> { | |
| try { | |
| const response = await this.get(`${this.basePath}/pages/${pageId}/description/`, { | |
| headers: { | |
| ...this.getHeader(), | |
| "Content-Type": "application/octet-stream", | |
| }, | |
| responseType: "arraybuffer", | |
| }); | |
| const data = response?.data; | |
| if (!Buffer.isBuffer(data)) { | |
| throw new Error("Expected response to be a Buffer"); | |
| } | |
| return data; | |
| } catch (error) { | |
| const appError = new AppError(error, { | |
| context: { operation: "fetchDescriptionBinary", pageId }, | |
| }); | |
| logger.error("Failed to fetch page description binary", appError); | |
| throw appError; | |
| } | |
| } | |
| async fetchDescriptionBinary(pageId: string): Promise<Buffer> { | |
| try { | |
| const response = await this.get(`${this.basePath}/pages/${pageId}/description/`, { | |
| headers: { | |
| ...this.getHeader(), | |
| "Content-Type": "application/octet-stream", | |
| }, | |
| responseType: "arraybuffer", | |
| }); | |
| const data = response?.data; | |
| if (Buffer.isBuffer(data)) { | |
| return data; | |
| } | |
| if (data instanceof ArrayBuffer) { | |
| return Buffer.from(data); | |
| } | |
| if (ArrayBuffer.isView(data)) { | |
| return Buffer.from(data.buffer, data.byteOffset, data.byteLength); | |
| } | |
| throw new Error("Expected response to be a Buffer or ArrayBuffer"); | |
| } catch (error) { | |
| const appError = new AppError(error, { | |
| context: { operation: "fetchDescriptionBinary", pageId }, | |
| }); | |
| logger.error("Failed to fetch page description binary", appError); | |
| throw appError; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@apps/live/src/services/page/core.service.ts` around lines 41 - 62,
fetchDescriptionBinary currently assumes axios returns a Node Buffer and throws
if Buffer.isBuffer fails; instead detect and convert ArrayBuffer or typed array
views to a Buffer before returning. In the fetchDescriptionBinary method
(responseType: "arraybuffer"), check if response.data is an ArrayBuffer or
ArrayBuffer view (use ArrayBuffer.isView) and create a Buffer from it
(Buffer.from(...)), then proceed with the Buffer.isBuffer check/return; keep the
existing error handling (AppError and logger) intact.
| import { defineConfig } from "vitest/config"; | ||
| import path from "path"; | ||
|
|
||
| export default defineConfig({ | ||
| test: { | ||
| environment: "node", | ||
| globals: true, | ||
| include: ["tests/**/*.test.ts", "tests/**/*.spec.ts"], | ||
| coverage: { | ||
| provider: "v8", | ||
| reporter: ["text", "json", "html"], | ||
| include: ["src/**/*.ts"], | ||
| exclude: ["src/**/*.d.ts", "src/**/types.ts"], | ||
| }, | ||
| }, | ||
| resolve: { | ||
| alias: { | ||
| "@": path.resolve(__dirname, "./src"), | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat apps/live/package.jsonRepository: makeplane/plane
Length of output: 2745
🏁 Script executed:
# Check if there are other vitest configs in the repo and how they handle path resolution
fd -e "vitest.config.ts" -e "vitest.config.js" -e "vite.config.ts" -e "vite.config.js" | head -20Repository: makeplane/plane
Length of output: 41
🏁 Script executed:
# Verify the specific file content
cat -n apps/live/vitest.config.tsRepository: makeplane/plane
Length of output: 684
🏁 Script executed:
# Check for other __dirname usages in ESM TypeScript files
rg "__dirname" --type ts --type tsx | head -20Repository: makeplane/plane
Length of output: 86
🏁 Script executed:
# Check if there are other vitest configs and how they're structured
fd -e "vitest.config.ts" -e "vitest.config.js" -e "vite.config.ts" -e "vite.config.js"Repository: makeplane/plane
Length of output: 41
🏁 Script executed:
# Specifically check if any ESM packages use __dirname
rg "type.*module" --type json | grep package.json | head -10Repository: makeplane/plane
Length of output: 551
🏁 Script executed:
# Check for __dirname usage in TypeScript files (use ts not tsx)
rg "__dirname" -t ts | head -30Repository: makeplane/plane
Length of output: 1476
🏁 Script executed:
# Find all vitest/vite config files
find . -name "vitest.config.*" -o -name "vite.config.*" | grep -v node_modules | head -20Repository: makeplane/plane
Length of output: 206
🏁 Script executed:
# Check if fileURLToPath is used elsewhere in the codebase as a pattern
rg "fileURLToPath" -t ts | head -10Repository: makeplane/plane
Length of output: 41
🏁 Script executed:
# Check if apps/web, apps/space, apps/admin are ESM or CommonJS
rg '"type".*"module"' apps/web/package.json apps/space/package.json apps/admin/package.json packages/codemods/package.jsonRepository: makeplane/plane
Length of output: 188
🏁 Script executed:
# Check the vitest config in packages/codemods
cat packages/codemods/vitest.config.tsRepository: makeplane/plane
Length of output: 179
🏁 Script executed:
# Check the package.json for apps/live to confirm the issue is isolated or widespread
cat apps/web/package.json | rg '"type"'
cat apps/space/package.json | rg '"type"'
cat apps/admin/package.json | rg '"type"'Repository: makeplane/plane
Length of output: 118
🏁 Script executed:
# Check if vite.config files in web/space/admin actually work or if they also have issues
cat apps/web/vite.config.ts | head -30Repository: makeplane/plane
Length of output: 1108
🏁 Script executed:
# Verify the proposed fix pattern is correct by checking Node.js ESM patterns
cat apps/live/vitest.config.ts | head -5Repository: makeplane/plane
Length of output: 170
🏁 Script executed:
# Search for correct ESM path handling patterns in the codebase
rg "fileURLToPath|import.meta.url" -t ts | head -20Repository: makeplane/plane
Length of output: 151
🏁 Script executed:
# Verify the proposed fix syntax is correct for Node.js ESM
# Check Node.js documentation for fileURLToPath usage
cat > /tmp/test_esm.mjs << 'EOF'
import { fileURLToPath } from "node:url";
const srcPath = fileURLToPath(new URL("./src", import.meta.url));
console.log("Path resolved:", srcPath);
EOF
# This verifies the syntax is valid (we can't run it in sandbox, but we can check the pattern)
echo "Proposed pattern is valid ESM syntax"Repository: makeplane/plane
Length of output: 95
🏁 Script executed:
# Check if the codebase has any similar correct ESM path resolution patterns we should follow
rg "fileURLToPath|new URL.*import.meta.url" -A 2 -B 2Repository: makeplane/plane
Length of output: 41
🏁 Script executed:
# Verify that __dirname specifically causes ReferenceError in ESM
# by checking if there are any comments or issues about it
rg "__dirname|ReferenceError.*__dirname" | head -10Repository: makeplane/plane
Length of output: 1058
🏁 Script executed:
# Check if vite-tsconfig-paths or other plugins polyfill __dirname
rg "global.__dirname|globalThis.__dirname" -t ts | head -10Repository: makeplane/plane
Length of output: 41
🏁 Script executed:
# Check if apps/web, apps/space, apps/admin actually work or have issues
# Look at the build scripts and see if there's a workaround
cat apps/web/package.json | grep -A 5 '"scripts"'Repository: makeplane/plane
Length of output: 391
🏁 Script executed:
# Most importantly: check if these config files have any special handling or if they're even used
cat apps/web/vite.config.ts | head -50Repository: makeplane/plane
Length of output: 1289
Fix __dirname usage in ESM config.
Line 18 relies on __dirname, which is undefined in ESM (this package has "type": "module"). This will throw a ReferenceError when Vitest loads the config. Use fileURLToPath with import.meta.url instead.
Proposed fix
-import path from "path";
+import { fileURLToPath } from "node:url";
export default defineConfig({
test: {
@@
resolve: {
alias: {
- "@": path.resolve(__dirname, "./src"),
+ "@": fileURLToPath(new URL("./src", import.meta.url)),
},
},
});📝 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.
| import { defineConfig } from "vitest/config"; | |
| import path from "path"; | |
| export default defineConfig({ | |
| test: { | |
| environment: "node", | |
| globals: true, | |
| include: ["tests/**/*.test.ts", "tests/**/*.spec.ts"], | |
| coverage: { | |
| provider: "v8", | |
| reporter: ["text", "json", "html"], | |
| include: ["src/**/*.ts"], | |
| exclude: ["src/**/*.d.ts", "src/**/types.ts"], | |
| }, | |
| }, | |
| resolve: { | |
| alias: { | |
| "@": path.resolve(__dirname, "./src"), | |
| }, | |
| }, | |
| }); | |
| import { defineConfig } from "vitest/config"; | |
| import { fileURLToPath } from "node:url"; | |
| export default defineConfig({ | |
| test: { | |
| environment: "node", | |
| globals: true, | |
| include: ["tests/**/*.test.ts", "tests/**/*.spec.ts"], | |
| coverage: { | |
| provider: "v8", | |
| reporter: ["text", "json", "html"], | |
| include: ["src/**/*.ts"], | |
| exclude: ["src/**/*.d.ts", "src/**/types.ts"], | |
| }, | |
| }, | |
| resolve: { | |
| alias: { | |
| "@": fileURLToPath(new URL("./src", import.meta.url)), | |
| }, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In `@apps/live/vitest.config.ts` around lines 1 - 21, The config uses __dirname in
the resolve.alias (path.resolve(__dirname, "./src")) which breaks under ESM;
replace this by deriving dirname from import.meta.url (use
fileURLToPath(import.meta.url) to get the file path then path.dirname) and pass
that dirname into path.resolve for the "@" alias; add the import for
fileURLToPath from "url" and compute a const dirname (or similar) at the top of
the module, then update the alias to use path.resolve(dirname, "./src").
Description
This PR fixes and stabilizes the PDF export functionality. It addresses issues where exports were either failing or producing incomplete/incorrect PDFs especially with images and custom nodes, and improves the overall reliability of the export flow. The changes ensure consistent rendering, proper data handling, and predictable output across different scenarios.
Type of Change
Screenshots and Media (if applicable)
N/A
Test Scenarios
References
N/A
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.