-
Notifications
You must be signed in to change notification settings - Fork 501
[Refactor] Update freestyle wrapper to use latest version and simplify code #1086
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0b550a1
285ea6c
75caac1
0e221ac
f99008a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Freestyle } from '@/lib/freestyle'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { emptyEmailTheme } from '@stackframe/stack-shared/dist/helpers/emails'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { getEnvVariable } from '@stackframe/stack-shared/dist/utils/env'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { captureError, StackAssertionError } from '@stackframe/stack-shared/dist/utils/errors'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { StackAssertionError } from '@stackframe/stack-shared/dist/utils/errors'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { bundleJavaScript } from '@stackframe/stack-shared/dist/utils/esbuild'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { get, has } from '@stackframe/stack-shared/dist/utils/objects'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Result } from "@stackframe/stack-shared/dist/utils/results"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -117,7 +117,14 @@ export async function renderEmailWithTemplate( | |||||||||||||||||||||||||||||||||||||||||||||||||
| `, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "/entry.js": deindent` | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { renderAll } from "./render.tsx"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| export default renderAll; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| export default async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const result = await renderAll(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return { _status: "ok", _data: result }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return { _status: "error", _error: String(e) }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| `, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }, { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| keepAsImports: ['arktype', 'react', 'react/jsx-runtime', '@react-email/components'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -136,21 +143,11 @@ export async function renderEmailWithTemplate( | |||||||||||||||||||||||||||||||||||||||||||||||||
| "@react-email/components": "0.1.1", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "arktype": "2.1.20", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const executeResult = await freestyle.executeScript(result.data, { nodeModules }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const executeResult = await freestyle.executeScript(result.data, { config: { nodeModules } }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (executeResult.status === "error") { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return Result.error(`${executeResult.error}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!executeResult.data.result) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const noResultError = new StackAssertionError("No result from Freestyle", { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| executeResult, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| templateOrDraftComponent, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| themeComponent, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| options, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| captureError("freestyle-no-result", noResultError); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| throw noResultError; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return Result.ok(executeResult.data.result as { html: string, text: string, subject: string, notificationCategory: string }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return Result.ok(executeResult.data as { html: string, text: string, subject: string, notificationCategory: string }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // unused, but kept for reference & in case we need it again | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -221,7 +218,14 @@ export async function renderEmailsWithTemplateBatched( | |||||||||||||||||||||||||||||||||||||||||||||||||
| `, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "/entry.js": deindent` | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { renderAll } from "./render.tsx"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| export default renderAll; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| export default async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const result = await renderAll(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return { _status: "ok", _data: result }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return { _status: "error", _error: String(e) }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| `, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }, { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| keepAsImports: ['arktype', 'react', 'react/jsx-runtime', '@react-email/components'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -240,21 +244,11 @@ export async function renderEmailsWithTemplateBatched( | |||||||||||||||||||||||||||||||||||||||||||||||||
| "@react-email/components": "0.1.1", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "arktype": "2.1.20", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const executeResult = await freestyle.executeScript(result.data, { nodeModules }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const executeResult = await freestyle.executeScript(result.data, { config: { nodeModules } }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (executeResult.status === "error") { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return Result.error(executeResult.error); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!executeResult.data.result) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const noResultError = new StackAssertionError("No result from Freestyle", { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| executeResult, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| templateOrDraftComponent, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| themeComponent, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| inputs, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| captureError("freestyle-no-result", noResultError); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| throw noResultError; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return Result.ok(executeResult.data.result as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return Result.ok(executeResult.data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
248
to
+251
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Add defensive validation before type casting. Similar to line 150, this casts the result without validating the data structure. Add defensive checks to ensure 🛡️ Proposed defensive validation if (executeResult.status === "error") {
return Result.error(executeResult.error);
}
- return Result.ok(executeResult.data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
+ const data = executeResult.data as any;
+ if (!Array.isArray(data)) {
+ throw new StackAssertionError("Freestyle returned non-array result for batched rendering", { data });
+ }
+ return Result.ok(data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| export type RenderEmailRequestForTenancy = { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -340,7 +334,14 @@ export async function renderEmailsForTenancyBatched(requests: RenderEmailRequest | |||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| files["/entry.js"] = deindent` | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { renderAll } from "./render.tsx"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| export default renderAll; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| export default async function() { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const result = await renderAll(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return { _status: "ok", _data: result }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return { _status: "error", _error: String(e) }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| `; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const bundle = await bundleJavaScript(files as Record<string, string> & { '/entry.js': string }, { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -362,20 +363,12 @@ export async function renderEmailsForTenancyBatched(requests: RenderEmailRequest | |||||||||||||||||||||||||||||||||||||||||||||||||
| "arktype": "2.1.20", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const execution = await freestyle.executeScript(bundle.data, { nodeModules }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const execution = await freestyle.executeScript(bundle.data, { config: { nodeModules } }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (execution.status === "error") { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return Result.error(execution.error); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!execution.data.result) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const noResultError = new StackAssertionError("No result from Freestyle", { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| execution, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| requests, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| captureError("freestyle-no-result", noResultError); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| throw noResultError; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| return Result.ok(execution.data.result as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return Result.ok(execution.data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
367
to
+371
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Add defensive validation before type casting. Same as line 251, add defensive checks to ensure the result is a valid array before casting. 🛡️ Proposed defensive validation if (execution.status === "error") {
return Result.error(execution.error);
}
- return Result.ok(execution.data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);
+ const data = execution.data as any;
+ if (!Array.isArray(data)) {
+ throw new StackAssertionError("Freestyle returned non-array result for tenancy batched rendering", { data });
+ }
+ return Result.ok(data as Array<{ html: string, text: string, subject?: string, notificationCategory?: string }>);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const findComponentValueUtil = `import React from 'react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Add defensive validation before type casting.
The code checks for error status but doesn't validate that
executeResult.dataactually contains the expected{ html, text, subject, notificationCategory }structure before casting. According to coding guidelines, prefer defensive coding with explicit error messages.🛡️ Proposed defensive validation
if (executeResult.status === "error") { return Result.error(`${executeResult.error}`); } - return Result.ok(executeResult.data as { html: string, text: string, subject: string, notificationCategory: string }); + const data = executeResult.data as any; + if (!data || typeof data.html !== 'string' || typeof data.text !== 'string') { + throw new StackAssertionError("Freestyle returned invalid email rendering result", { data }); + } + return Result.ok(data as { html: string, text: string, subject?: string, notificationCategory?: string });🤖 Prompt for AI Agents