-
Notifications
You must be signed in to change notification settings - Fork 59.3k
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
[Artifacts] add preview html code #5092
Conversation
@lloydzhou is attempting to deploy a commit to the NextChat Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe latest updates significantly enhance the application's capabilities by introducing new components for managing and sharing HTML artifacts. The Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
app/components/markdown.tsx (1)
63-64
: Consider handling iframe clicks.Currently, the
onClick
handler logs a message to the console. Consider implementing a more meaningful action or removing the handler if it's not needed.- onClick={() => console.log("click")} + onClick={() => handleIframeClick()}
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/components/markdown.tsx (2 hunks)
Additional comments not posted (3)
app/components/markdown.tsx (3)
78-79
: Verify the sandbox attribute for potential security issues.The
sandbox
attribute is set toallow-scripts
. Ensure that this does not introduce any security vulnerabilities, as it allows scripts to run within the iframe.
Line range hint
148-148
:
LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
_MarkDownContent
are updated to handle the new functionality.Verification successful
No updates necessary for function usage.
The
_MarkDownContent
function is not used elsewhere in the codebase, so no updates are required to handle the new functionality.
app/components/markdown.tsx
:_MarkDownContent
is defined and exported but not used elsewhere.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_MarkDownContent` are updated to handle the new functionality. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type js --type ts --type tsx --type jsx --pattern '_MarkDownContent'Length of output: 485
Script:
#!/bin/bash # Description: Verify all function calls to `_MarkDownContent` are updated to handle the new functionality. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg '_MarkDownContent' --type js --type ts --type tsx --type jsx -A 3Length of output: 96
Script:
#!/bin/bash # Description: Verify all function calls to `_MarkDownContent` are updated to handle the new functionality. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg '_MarkDownContent' --type js --type ts --type jsx -A 3Length of output: 85
Script:
#!/bin/bash # Description: Verify all function calls to `_MarkDownContent` are updated to handle the new functionality. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg '_MarkDownContent' --type js --type ts -A 3Length of output: 554
90-90
: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
PreCode
are updated to handle the new functionality.Verification successful
LGTM! But verify the function usage in the codebase.
The code changes are approved.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `PreCode` are updated to handle the new functionality. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type js --type ts --type tsx --type jsx --pattern 'PreCode'Length of output: 476
Script:
#!/bin/bash # Description: Verify all function calls to `PreCode` are updated to handle the new functionality. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg 'PreCode' --type js --type ts --type tsx --type jsxLength of output: 82
Script:
#!/bin/bash # Description: Verify all function calls to `PreCode` are updated to handle the new functionality. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg 'PreCode' --type js --type ts --type jsxLength of output: 71
Script:
#!/bin/bash # Description: Verify all function calls to `PreCode` are updated to handle the new functionality. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg 'PreCode' --type js --type tsLength of output: 162
Script:
#!/bin/bash # Description: Verify the usage of `PreCode` in `app/components/markdown.tsx` to ensure it handles the new functionality. # Extract lines around the usage of `PreCode` rg 'PreCode' -A 10 -B 10 app/components/markdown.tsxLength of output: 1270
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.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (2)
app/components/markdown.tsx (2)
63-64
: Remove unusedref
.The
ref
variable is declared but not used within the component.- const ref = useRef<HTMLDivElement>(null);
74-74
: Improve theonClick
handler.The
onClick
handler currently logs a message to the console. Consider implementing a more meaningful action or removing the handler if it's not needed.- onClick={() => console.log("click")} + onClick={() => {/* Implement meaningful action or remove */}}
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/components/markdown.tsx (2 hunks)
Additional comments not posted (3)
app/components/markdown.tsx (3)
90-90
: Verify debounce delay.Ensure that the 600ms debounce delay is appropriate for the use case.
114-114
: Verify the rendering logic.Ensure that the rendering logic for
HTMLPreview
andMermaid
components works as expected.Verification successful
Rendering logic verified.
The rendering logic for both
HTMLPreview
andMermaid
components is correctly implemented inapp/components/markdown.tsx
. Both components are conditionally rendered based on the length of their respective code strings.
HTMLPreview
is rendered whenhtmlCode.length > 0
.Mermaid
is rendered withmermaidCode
as a prop.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the rendering logic for `HTMLPreview` and `Mermaid` components. # Test: Search for the rendering logic. Expect: Correct rendering of both components. rg --type python -A 5 $'HTMLPreview'Length of output: 66
Script:
#!/bin/bash # Description: Verify the rendering logic for `HTMLPreview` and `Mermaid` components. # Search for the rendering logic of `HTMLPreview` in TypeScript files. rg --type tsx -A 5 'HTMLPreview' # Search for the rendering logic of `Mermaid` in TypeScript files. rg --type tsx -A 5 'Mermaid'Length of output: 117
Script:
#!/bin/bash # Description: Verify the rendering logic for `HTMLPreview` and `Mermaid` components. # Search for the rendering logic of `HTMLPreview` in TypeScript files. rg --type typescript -A 5 'HTMLPreview' # Search for the rendering logic of `Mermaid` in TypeScript files. rg --type typescript -A 5 'Mermaid'Length of output: 145
Script:
#!/bin/bash # Description: Verify the rendering logic for `HTMLPreview` and `Mermaid` components. # Search for the rendering logic of `HTMLPreview` in TypeScript React files. rg --type tsx -A 5 'HTMLPreview' # Search for the rendering logic of `Mermaid` in TypeScript React files. rg --type tsx -A 5 'Mermaid'Length of output: 117
Script:
#!/bin/bash # Description: Verify the rendering logic for `HTMLPreview` and `Mermaid` components. # Search for the rendering logic of `HTMLPreview` in files with .tsx extension. rg 'HTMLPreview' --glob '*.tsx' -A 5 # Search for the rendering logic of `Mermaid` in files with .tsx extension. rg 'Mermaid' --glob '*.tsx' -A 5Length of output: 2673
Line range hint
153-153
:
Verify integration withPreCode
.Ensure that the integration of
_MarkDownContent
withPreCode
works seamlessly.Verification successful
Verify integration with
PreCode
.The integration of
_MarkDownContent
withPreCode
is confirmed to be seamless within the markdown rendering context.
PreCode
is defined and utilized inapp/components/markdown.tsx
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of `_MarkDownContent` with `PreCode`. # Test: Search for the integration points. Expect: Seamless integration. rg --type python -A 5 $'_MarkDownContent'Length of output: 71
Script:
#!/bin/bash # Description: Verify the integration of `_MarkDownContent` with `PreCode`. # Test: Search for the integration points in TypeScript/TSX files. Expect: Seamless integration. rg --type tsx -A 5 '_MarkDownContent'Length of output: 65
Script:
#!/bin/bash # Description: Verify the integration of `_MarkDownContent` with `PreCode`. # Test: Search for the integration points in TypeScript/TSX files. Expect: Seamless integration. rg --type typescript -A 5 '_MarkDownContent'Length of output: 79
Script:
#!/bin/bash # Description: Verify the integration of `_MarkDownContent` with `PreCode`. # Test: Search for the integration points in TypeScript/TSX files. Expect: Seamless integration. rg --type ts -A 5 '_MarkDownContent'Length of output: 712
Script:
#!/bin/bash # Description: Verify the integration of `_MarkDownContent` with `PreCode`. # Test: Search for `PreCode` usage in TypeScript/TSX files. Expect: Seamless integration. rg --type ts -A 5 'PreCode'Length of output: 873
Your build has completed! |
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.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/components/markdown.tsx (3 hunks)
Additional comments not posted (4)
app/components/markdown.tsx (4)
16-16
: Import statement fornanoid
looks good.The
nanoid
package is correctly imported for generating unique IDs.
118-118
: State management forhtmlCode
looks good.The new state variable
htmlCode
is correctly added to manage the HTML code.
133-133
:useEffect
hook setup looks good.The
useEffect
hook is correctly set up to callrenderArtifacts
.
154-154
: Rendering logic forHTMLPreview
looks good.The
HTMLPreview
component is correctly rendered based on thehtmlCode
state.
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/components/markdown.tsx (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/components/markdown.tsx
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.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- app/api/artifact/route.ts (1 hunks)
- app/components/artifact.tsx (1 hunks)
- app/components/home.tsx (3 hunks)
- app/components/markdown.tsx (3 hunks)
- app/config/server.ts (1 hunks)
- app/constant.ts (3 hunks)
- app/locales/cn.ts (1 hunks)
- app/locales/en.ts (1 hunks)
Additional comments not posted (15)
app/api/artifact/route.ts (2)
1-3
: Imports look good.The imported modules are appropriate for the functionality provided in the file.
45-48
: Default response handling looks good.The function appropriately handles invalid requests by returning a 400 status.
app/config/server.ts (2)
161-163
: New configuration parameters look good.The new parameters for Cloudflare are correctly added to the
getServerSideConfig
function.
Line range hint
1-164
: Function logic looks good.The function correctly handles the new parameters and maintains its overall logic.
app/components/home.tsx (3)
42-44
: Dynamic import looks good.The
Artifact
component is appropriately imported dynamically with a loading state.
Line range hint
132-150
: Routing logic looks good.The routing logic appropriately includes a new route for the
Artifact
component.
Line range hint
1-150
: Function logic looks good.The function correctly handles the new routing logic and maintains its overall logic.
app/components/markdown.tsx (2)
70-79
: Optimize therenderArtifacts
function.The
renderArtifacts
function can be optimized to avoid querying the DOM multiple times.- const mermaidDom = ref.current.querySelector("code.language-mermaid"); - if (mermaidDom) { - setMermaidCode((mermaidDom as HTMLElement).innerText); - } - const htmlDom = ref.current.querySelector("code.language-html"); - if (htmlDom) { - setHtmlCode((htmlDom as HTMLElement).innerText); - } + const codeElements = ref.current.querySelectorAll("code"); + codeElements.forEach((element) => { + if (element.classList.contains("language-mermaid")) { + setMermaidCode(element.innerText); + } else if (element.classList.contains("language-html")) { + setHtmlCode(element.innerText); + } + });
Line range hint
16-39
:
Enhance iframe security and cleanup event listener.The
HTMLPreview
component correctly renders an HTML preview using an iframe. However, consider enhancing the iframe security and ensuring proper cleanup of the event listener.
Enhance iframe security:
Add more restrictive values to thesandbox
attribute and consider using a CSP header.- sandbox="allow-forms allow-modals allow-scripts" + sandbox="allow-forms allow-modals allow-scripts allow-same-origin" + csp="default-src 'self'; script-src 'none';"Cleanup event listener:
Ensure that the event listener is properly cleaned up to avoid potential memory leaks.+ useEffect(() => { + const handleMessage = (e) => { + const { id, height } = e.data; + if (id == frameId.current) { + console.log("setHeight", height); + if (height < 600) { + setHeight(height + 40); + } + } + }; + window.addEventListener("message", handleMessage); + return () => { + window.removeEventListener("message", handleMessage); + }; + }, []);app/components/artifact.tsx (1)
16-39
: Enhance iframe security and cleanup event listener.The
HTMLPreview
component correctly renders an HTML preview using an iframe. However, consider enhancing the iframe security and ensuring proper cleanup of the event listener.
Enhance iframe security:
Add more restrictive values to thesandbox
attribute and consider using a CSP header.- sandbox="allow-forms allow-modals allow-scripts" + sandbox="allow-forms allow-modals allow-scripts allow-same-origin" + csp="default-src 'self'; script-src 'none';"Cleanup event listener:
Ensure that the event listener is properly cleaned up to avoid potential memory leaks.+ useEffect(() => { + const handleMessage = (e) => { + const { id, height } = e.data; + if (id == frameId.current) { + console.log("setHeight", height); + if (height < 600) { + setHeight(height + 40); + } + } + }; + window.addEventListener("message", handleMessage); + return () => { + window.removeEventListener("message", handleMessage); + }; + }, []);app/constant.ts (3)
11-11
: Ensure thePREVIEW_URL
is correct and secure.Verify that the
PREVIEW_URL
is correct and uses a secure protocol (https).Verification successful
The
PREVIEW_URL
is correct and uses a secure protocol (HTTPS).
- Location:
app/constant.ts:11
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the PREVIEW_URL is correct and uses a secure protocol. # Test: Check the PREVIEW_URL value. Expect: The URL uses HTTPS. rg --type js --pattern 'export const PREVIEW_URL = "https://app.nextchat.dev"'Length of output: 492
Script:
#!/bin/bash # Description: Verify the PREVIEW_URL is correct and uses a secure protocol. # Test: Check the PREVIEW_URL value. Expect: The URL uses HTTPS. rg 'export const PREVIEW_URL = "https://app.nextchat.dev"'Length of output: 131
35-35
: Ensure the newPath.Artifact
is used correctly.Verify that the new
Path.Artifact
is used correctly in the codebase.Verification successful
The new
Path.Artifact
is used correctly.The
Path.Artifact
is utilized in the following files:
app/components/home.tsx
- Line:
const isArtifact = location.pathname.includes(Path.Artifact);
app/components/artifact.tsx
- Lines:
[location.origin, "#", Path.Artifact, "/", name].join("")
- Lines:
fetch(ApiPath.Artifact, {
- Lines:
fetch(\
${ApiPath.Artifact}?id=${id}`)`These usages involve pathname checks and URL construction, which are appropriate for a path constant.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new Path.Artifact is used correctly in the codebase. # Test: Search for the usage of Path.Artifact. Expect: Correct usage in the codebase. rg --type js --pattern 'Path.Artifact'Length of output: 450
Script:
#!/bin/bash # Description: Verify the new Path.Artifact is used correctly in the codebase. # Test: Search for the usage of Path.Artifact. Expect: Correct usage in the codebase. rg 'Path.Artifact'Length of output: 327
47-47
: Ensure the newApiPath.Artifact
is used correctly.Verify that the new
ApiPath.Artifact
is used correctly in the codebase.Verification successful
The new
ApiPath.Artifact
is used correctly.The
ApiPath.Artifact
is used infetch
calls withinapp/components/artifact.tsx
, which is appropriate for an API path.
app/components/artifact.tsx
:
fetch(ApiPath.Artifact, {
fetch(\
${ApiPath.Artifact}?id=${id}`)`Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new ApiPath.Artifact is used correctly in the codebase. # Test: Search for the usage of ApiPath.Artifact. Expect: Correct usage in the codebase. rg --type js --pattern 'ApiPath.Artifact'Length of output: 453
Script:
#!/bin/bash # Description: Verify the new ApiPath.Artifact is used correctly in the codebase. # Test: Search for the usage of ApiPath.Artifact. Expect: Correct usage in the codebase. rg 'ApiPath.Artifact'Length of output: 151
app/locales/cn.ts (1)
107-110
: LGTM!The new entries for
Artifact
translations are correct and consistent with the rest of the file.app/locales/en.ts (1)
109-112
: LGTM!The new entries for
Artifact
translations are correct and consistent with the rest of the file.
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.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- app/api/artifact/route.ts (1 hunks)
- app/components/artifact.tsx (1 hunks)
- app/components/home.tsx (3 hunks)
Files skipped from review due to trivial changes (1)
- app/api/artifact/route.ts
Files skipped from review as they are similar to previous changes (1)
- app/components/home.tsx
Additional context used
Biome
app/components/artifact.tsx
[error] 70-70: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Additional comments not posted (4)
app/components/artifact.tsx (4)
66-66
: Review iframe sandbox attributes for security.The iframe's
sandbox
attribute should be reviewed to ensure it includes only necessary permissions. Consider addingallow-same-origin
if needed, but be cautious of security implications.
92-108
: Improve error handling inupload
function.The
upload
function should provide more detailed error messages to help with debugging..catch((e) => { console.error("Artifact upload failed:", e); showToast(Locale.Export.Artifact.Error); });
173-179
: Improve error handling inuseEffect
hook.The
useEffect
hook should handle errors when fetching the artifact..then((res) => { if (!res.ok) { throw new Error("Network response was not ok"); } return res.text(); }) .then(setCode) .catch((error) => { console.error("Failed to fetch artifact:", error); });
204-215
: Ensure loading state is correctly managed.The loading state should be correctly managed to ensure the loading indicator is shown and hidden appropriately.
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.
Actionable comments posted: 5
Outside diff range, codebase verification and nitpick comments (2)
app/components/artifact.tsx (1)
71-71
: Use optional chaining forprops?.onLoad
.The
props?.onLoad
can be simplified using optional chaining.onLoad={(e) => props?.onLoad?.(title)}Tools
Biome
[error] 71-71: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
app/components/ui-lib.tsx (1)
Line range hint
446-459
: Enhance type safety and handle edge cases inSelector
component.The
defaultSelectedValue
prop should be properly typed and edge cases should be handled to ensure robustness.defaultSelectedValue?: T[] | T; onSelection?: (selection: T[]) => void; onClose?: () => void; multiple?: boolean; }) { const selected = props.multiple ? Array.isArray(props.defaultSelectedValue) && props.defaultSelectedValue.includes(item.value) : props.defaultSelectedValue === item.value;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- app/components/artifact.tsx (1 hunks)
- app/components/chat.tsx (4 hunks)
- app/components/markdown.tsx (3 hunks)
- app/components/ui-lib.tsx (2 hunks)
- app/constant.ts (4 hunks)
- app/locales/cn.ts (2 hunks)
- app/locales/en.ts (2 hunks)
- app/store/mask.ts (2 hunks)
Files skipped from review as they are similar to previous changes (4)
- app/components/markdown.tsx
- app/constant.ts
- app/locales/cn.ts
- app/locales/en.ts
Additional context used
Biome
app/components/artifact.tsx
[error] 71-71: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Additional comments not posted (6)
app/store/mask.ts (2)
5-5
: Import statement looks good.The import statement for
Plugin
is necessary for the newplugin
property in theMask
type definition.
20-20
: Type definition update looks good.The addition of the optional
plugin
property enhances the flexibility of theMask
type.app/components/chat.tsx (4)
40-40
: Import statement for PluginIcon looks good.The import statement aligns with the changes described in the summary.
481-481
: State variableshowPluginSelector
addition looks good.The state variable is correctly initialized using
useState
and follows the pattern of other state variables in the component.
627-631
: NewChatAction
component for plugin selector looks good.The
ChatAction
component is correctly added to trigger the plugin selector.
632-652
: Plugin selector integration looks good, but verify plugin selection logic.The
Selector
component is correctly integrated and theonSelection
callback updates the current session's mask with the selected plugin(s). Ensure that the plugin selection logic works as expected.Verification successful
Plugin selection logic verified.
The
onSelection
callback correctly updates the session's mask with the selected plugin(s) and displays a toast notification.
app/components/chat.tsx
: Lines 632-652Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the plugin selection logic in the codebase. # Test: Search for the function usage. Expect: Correct handling of plugin selection. rg --type javascript -A 10 $'onSelection'Length of output: 75
Script:
#!/bin/bash # Description: Verify the plugin selection logic in the codebase. # Test: Search for the function usage. Expect: Correct handling of plugin selection. rg --type typescript -A 10 $'onSelection'Length of output: 75
Script:
#!/bin/bash # Description: Verify the plugin selection logic in the codebase. # Test: Search for the function usage. Expect: Correct handling of plugin selection. rg --type tsx -A 10 $'onSelection'Length of output: 61
Script:
#!/bin/bash # Description: Verify the plugin selection logic in the codebase. # Test: Search for the function usage. Expect: Correct handling of plugin selection. rg -A 10 $'onSelection'Length of output: 2676
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/components/ui-lib.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/components/ui-lib.tsx
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.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/components/artifact.tsx (1 hunks)
Additional context used
Biome
app/components/artifact.tsx
[error] 73-73: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
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.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
app/components/ui-lib.tsx (1)
Line range hint
452-465
:
Avoid using@ts-ignore
and handle type checking properly.The usage of
@ts-ignore
should be avoided. Instead, implement proper type checking to handle the case wheredefaultSelectedValue
is an array or a single value.- ? // @ts-ignore - props.defaultSelectedValue?.includes(item.value) + ? Array.isArray(props.defaultSelectedValue) + ? props.defaultSelectedValue.includes(item.value) + : false
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/components/markdown.tsx (3 hunks)
- app/components/ui-lib.tsx (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/components/markdown.tsx
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.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/components/artifact.tsx (1 hunks)
Additional context used
Biome
app/components/artifact.tsx
[error] 73-73: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
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.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- app/api/artifact/route.ts (1 hunks)
- app/components/artifact.tsx (1 hunks)
- app/config/server.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- app/api/artifact/route.ts
- app/config/server.ts
Additional context used
Biome
app/components/artifact.tsx
[error] 73-73: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
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.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- app/api/artifact/route.ts (1 hunks)
- app/components/chat.tsx (4 hunks)
- app/components/home.tsx (3 hunks)
- app/components/ui-lib.tsx (4 hunks)
- app/config/server.ts (1 hunks)
- app/constant.ts (3 hunks)
- app/locales/cn.ts (2 hunks)
- app/locales/en.ts (2 hunks)
Files skipped from review as they are similar to previous changes (6)
- app/components/chat.tsx
- app/components/home.tsx
- app/config/server.ts
- app/constant.ts
- app/locales/cn.ts
- app/locales/en.ts
Additional context used
Biome
app/api/artifact/route.ts
[error] 18-18: Don't use 'Number' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'number' instead(lint/complexity/noBannedTypes)
Additional comments not posted (7)
app/api/artifact/route.ts (6)
1-4
: LGTM!The imports and initial setup are correct.
12-50
: Ensure proper error handling for the POST method.The POST method handles data storage but lacks error handling for the
fetch
call.Consider adding a try-catch block to handle potential errors during the fetch operation.
if (req.method === "POST") { try { const clonedBody = await req.text(); const hashedCode = md5.hash(clonedBody).trim(); const res = await fetch(storeUrl(hashedCode), { headers: storeHeaders(), method: "PUT", body: clonedBody, }); const result = await res.json(); console.log("save data", result); if (result?.success) { return NextResponse.json( { code: 0, id: hashedCode, result }, { status: res.status }, ); } return NextResponse.json( { error: true, msg: "Save data error" }, { status: 400 }, ); } catch (error) { console.error("Error saving data:", error); return NextResponse.json( { error: true, msg: "Internal server error" }, { status: 500 }, ); } }Tools
Biome
[error] 18-18: Don't use 'Number' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'number' instead(lint/complexity/noBannedTypes)
52-63
: Ensure proper error handling for the GET method.The GET method handles data retrieval but lacks error handling for the
fetch
call.Consider adding a try-catch block to handle potential errors during the fetch operation.
if (req.method === "GET") { try { const id = req?.nextUrl?.searchParams?.get("id"); const res = await fetch(storeUrl(id), { headers: storeHeaders(), method: "GET", }); return new Response(res.body, { status: res.status, statusText: res.statusText, headers: res.headers, }); } catch (error) { console.error("Error retrieving data:", error); return NextResponse.json( { error: true, msg: "Internal server error" }, { status: 500 }, ); } }
64-67
: LGTM!The fallback handling for invalid requests is correct.
70-71
: LGTM!The POST and GET methods are exported correctly.
73-73
: LGTM!The runtime is set to "edge" and exported correctly.
app/components/ui-lib.tsx (1)
Line range hint
468-481
:
LGTM!The changes to the
Selector
component enhance its versatility and correctly handle multiple selections.
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.
Actionable comments posted: 6
Outside diff range, codebase verification and nitpick comments (2)
app/components/artifact.tsx (1)
1-1
: Remove unused import.The
useWindowSize
import is not used in this file and can be removed.- import { useWindowSize } from "@/app/utils";
app/components/ui-lib.tsx (1)
Line range hint
468-481
:
Ensure type safety fordefaultSelectedValue
.The
defaultSelectedValue
prop should be type-checked to ensure it is an array whenmultiple
is true.const selected = props.multiple ? Array.isArray(props.defaultSelectedValue) && props.defaultSelectedValue.includes(item.value) : props.defaultSelectedValue === item.value;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/components/artifact.tsx (1 hunks)
- app/components/ui-lib.tsx (4 hunks)
Additional context used
Biome
app/components/artifact.tsx
[error] 77-77: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Additional comments not posted (1)
app/components/artifact.tsx (1)
35-47
: Ensure proper cleanup of event listeners.The
useEffect
hook adds an event listener to the window but does not remove it during cleanup. This can lead to memory leaks.useEffect(() => { const handleMessage = (e) => { const { id, height, title } = e.data; setTitle(title); if (id == frameId.current) { setIframeHeight(height); } }; window.addEventListener("message", handleMessage); return () => { window.removeEventListener("message", handleMessage); }; }, []);Likely invalid or redundant comment.
β¦atGPT-Next-Web into feature/artifacts-style
Feature/artifacts style
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- app/api/artifact/route.ts (1 hunks)
- app/components/artifact.module.scss (1 hunks)
- app/components/artifact.tsx (1 hunks)
- app/components/chat.tsx (4 hunks)
- app/components/markdown.tsx (3 hunks)
- app/components/ui-lib.module.scss (1 hunks)
- app/components/ui-lib.tsx (5 hunks)
Files skipped from review due to trivial changes (1)
- app/components/ui-lib.module.scss
Files skipped from review as they are similar to previous changes (4)
- app/api/artifact/route.ts
- app/components/artifact.tsx
- app/components/chat.tsx
- app/components/markdown.tsx
Additional comments not posted (9)
app/components/artifact.module.scss (5)
1-5
: LGTM!The styles for the
.artifact
container are clear and well-structured.
6-12
: LGTM!The styles for the
.artifact-header
are well-defined and appropriate.
13-18
: LGTM!The styles for the
.artifact-title
are clear and appropriate.
19-23
: LGTM!The styles for the
.artifact-content
are well-defined and appropriate.
26-30
: LGTM!The styles for the
.artifact-iframe
are clear and appropriate.app/components/ui-lib.tsx (4)
22-23
: LGTM!The new imports
useCallback
anduseRef
are necessary for theFullScreen
component.
468-479
: LGTM!The changes to the
defaultSelectedValue
prop type and the updated logic for handling multiple selections enhance theSelector
component's versatility.
481-494
: LGTM!The
handleSelection
function is appropriately updated to handle both single and multiple selections.
538-572
: Clean up the event listener and define it inside theuseEffect
hook.The
fullscreenchange
event listener should be cleaned up on component unmount. Additionally, define the event listener inside theuseEffect
hook to avoid adding multiple listeners.useEffect(() => { const handleScreenChange = (e: any) => { if (e.target === ref.current) { setFullScreen(!!document.fullscreenElement); } }; document.addEventListener("fullscreenchange", handleScreenChange); return () => { document.removeEventListener("fullscreenchange", handleScreenChange); }; }, []);Likely invalid or redundant comment.
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/components/artifact.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/components/artifact.tsx
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/components/artifact.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/components/artifact.tsx
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.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- app/api/artifacts/route.ts (1 hunks)
- app/components/artifacts.module.scss (1 hunks)
- app/components/artifacts.tsx (1 hunks)
- app/components/chat.tsx (4 hunks)
- app/components/home.tsx (3 hunks)
- app/components/markdown.tsx (3 hunks)
- app/constant.ts (3 hunks)
- app/locales/cn.ts (2 hunks)
- app/locales/en.ts (2 hunks)
- app/store/mask.ts (3 hunks)
Files skipped from review due to trivial changes (1)
- app/components/artifacts.module.scss
Files skipped from review as they are similar to previous changes (7)
- app/components/chat.tsx
- app/components/home.tsx
- app/components/markdown.tsx
- app/constant.ts
- app/locales/cn.ts
- app/locales/en.ts
- app/store/mask.ts
Additional comments not posted (5)
app/api/artifacts/route.ts (5)
1-4
: LGTM!The import statements and configuration retrieval are correct.
31-50
: LGTM!The API call and response handling are correct.
52-63
: LGTM!The GET request handling is correct.
64-73
: LGTM!The default response for invalid requests and export statements are correct.
12-30
: Improve error handling and variable naming.
- The error handling can be improved by providing more specific error messages.
- The variable
res
is re-declared within the same scope, which can cause confusion. Consider renaming it to avoid conflicts.- const res = await fetch(`${storeUrl()}/bulk`, { + const response = await fetch(`${storeUrl()}/bulk`, {Likely invalid or redundant comment.
export function HTMLPreview(props: { | ||
code: string; | ||
autoHeight?: boolean; | ||
height?: number | string; | ||
onLoad?: (title?: string) => void; | ||
}) { | ||
const ref = useRef<HTMLIFrameElement>(null); | ||
const frameId = useRef<string>(nanoid()); | ||
const [iframeHeight, setIframeHeight] = useState(600); | ||
const [title, setTitle] = useState(""); | ||
/* | ||
* https://stackoverflow.com/questions/19739001/what-is-the-difference-between-srcdoc-and-src-datatext-html-in-an | ||
* 1. using srcdoc | ||
* 2. using src with dataurl: | ||
* easy to share | ||
* length limit (Data URIs cannot be larger than 32,768 characters.) | ||
*/ | ||
|
||
useEffect(() => { | ||
const handleMessage = (e: any) => { | ||
const { id, height, title } = e.data; | ||
setTitle(title); | ||
if (id == frameId.current) { | ||
setIframeHeight(height); | ||
} | ||
}; | ||
window.addEventListener("message", handleMessage); | ||
return () => { | ||
window.removeEventListener("message", handleMessage); | ||
}; | ||
}, []); | ||
|
||
const height = useMemo(() => { | ||
if (!props.autoHeight) return props.height || 600; | ||
if (typeof props.height === "string") { | ||
return props.height; | ||
} | ||
const parentHeight = props.height || 600; | ||
return iframeHeight + 40 > parentHeight ? parentHeight : iframeHeight + 40; | ||
}, [props.autoHeight, props.height, iframeHeight]); | ||
|
||
const srcDoc = useMemo(() => { | ||
const script = `<script>new ResizeObserver((entries) => parent.postMessage({id: '${frameId.current}', height: entries[0].target.clientHeight}, '*')).observe(document.body)</script>`; | ||
if (props.code.includes("</head>")) { | ||
props.code.replace("</head>", "</head>" + script); | ||
} | ||
return props.code + script; | ||
}, [props.code]); | ||
|
||
const handleOnLoad = () => { | ||
if (props?.onLoad) { | ||
props.onLoad(title); | ||
} | ||
}; | ||
|
||
return ( | ||
<iframe | ||
className={styles["artifacts-iframe"]} | ||
id={frameId.current} | ||
ref={ref} | ||
sandbox="allow-forms allow-modals allow-scripts" | ||
style={{ height }} | ||
srcDoc={srcDoc} | ||
onLoad={handleOnLoad} | ||
/> | ||
); | ||
} |
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.
Consider using useCallback
and handling potential XSS vulnerabilities.
- Use
useCallback
to memoize thehandleOnLoad
function. - Ensure that the HTML code passed to the iframe is sanitized to prevent XSS attacks.
+ import { useCallback } from "react";
- const handleOnLoad = () => {
+ const handleOnLoad = useCallback(() => {
if (props?.onLoad) {
props.onLoad(title);
}
- };
+ }, [props?.onLoad, title]);
+ const sanitizedCode = sanitizeHTML(props.code); // Implement sanitizeHTML function to sanitize the HTML code
- return props.code + script;
+ return sanitizedCode + script;
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.
export function HTMLPreview(props: { | |
code: string; | |
autoHeight?: boolean; | |
height?: number | string; | |
onLoad?: (title?: string) => void; | |
}) { | |
const ref = useRef<HTMLIFrameElement>(null); | |
const frameId = useRef<string>(nanoid()); | |
const [iframeHeight, setIframeHeight] = useState(600); | |
const [title, setTitle] = useState(""); | |
/* | |
* https://stackoverflow.com/questions/19739001/what-is-the-difference-between-srcdoc-and-src-datatext-html-in-an | |
* 1. using srcdoc | |
* 2. using src with dataurl: | |
* easy to share | |
* length limit (Data URIs cannot be larger than 32,768 characters.) | |
*/ | |
useEffect(() => { | |
const handleMessage = (e: any) => { | |
const { id, height, title } = e.data; | |
setTitle(title); | |
if (id == frameId.current) { | |
setIframeHeight(height); | |
} | |
}; | |
window.addEventListener("message", handleMessage); | |
return () => { | |
window.removeEventListener("message", handleMessage); | |
}; | |
}, []); | |
const height = useMemo(() => { | |
if (!props.autoHeight) return props.height || 600; | |
if (typeof props.height === "string") { | |
return props.height; | |
} | |
const parentHeight = props.height || 600; | |
return iframeHeight + 40 > parentHeight ? parentHeight : iframeHeight + 40; | |
}, [props.autoHeight, props.height, iframeHeight]); | |
const srcDoc = useMemo(() => { | |
const script = `<script>new ResizeObserver((entries) => parent.postMessage({id: '${frameId.current}', height: entries[0].target.clientHeight}, '*')).observe(document.body)</script>`; | |
if (props.code.includes("</head>")) { | |
props.code.replace("</head>", "</head>" + script); | |
} | |
return props.code + script; | |
}, [props.code]); | |
const handleOnLoad = () => { | |
if (props?.onLoad) { | |
props.onLoad(title); | |
} | |
}; | |
return ( | |
<iframe | |
className={styles["artifacts-iframe"]} | |
id={frameId.current} | |
ref={ref} | |
sandbox="allow-forms allow-modals allow-scripts" | |
style={{ height }} | |
srcDoc={srcDoc} | |
onLoad={handleOnLoad} | |
/> | |
); | |
} | |
import { useCallback } from "react"; | |
export function HTMLPreview(props: { | |
code: string; | |
autoHeight?: boolean; | |
height?: number | string; | |
onLoad?: (title?: string) => void; | |
}) { | |
const ref = useRef<HTMLIFrameElement>(null); | |
const frameId = useRef<string>(nanoid()); | |
const [iframeHeight, setIframeHeight] = useState(600); | |
const [title, setTitle] = useState(""); | |
/* | |
* https://stackoverflow.com/questions/19739001/what-is-the-difference-between-srcdoc-and-src-datatext-html-in-an | |
* 1. using srcdoc | |
* 2. using src with dataurl: | |
* easy to share | |
* length limit (Data URIs cannot be larger than 32,768 characters.) | |
*/ | |
useEffect(() => { | |
const handleMessage = (e: any) => { | |
const { id, height, title } = e.data; | |
setTitle(title); | |
if (id == frameId.current) { | |
setIframeHeight(height); | |
} | |
}; | |
window.addEventListener("message", handleMessage); | |
return () => { | |
window.removeEventListener("message", handleMessage); | |
}; | |
}, []); | |
const height = useMemo(() => { | |
if (!props.autoHeight) return props.height || 600; | |
if (typeof props.height === "string") { | |
return props.height; | |
} | |
const parentHeight = props.height || 600; | |
return iframeHeight + 40 > parentHeight ? parentHeight : iframeHeight + 40; | |
}, [props.autoHeight, props.height, iframeHeight]); | |
const srcDoc = useMemo(() => { | |
const script = `<script>new ResizeObserver((entries) => parent.postMessage({id: '${frameId.current}', height: entries[0].target.clientHeight}, '*')).observe(document.body)</script>`; | |
if (props.code.includes("</head>")) { | |
props.code.replace("</head>", "</head>" + script); | |
} | |
const sanitizedCode = sanitizeHTML(props.code); // Implement sanitizeHTML function to sanitize the HTML code | |
return sanitizedCode + script; | |
}, [props.code]); | |
const handleOnLoad = useCallback(() => { | |
if (props?.onLoad) { | |
props.onLoad(title); | |
} | |
}, [props?.onLoad, title]); | |
return ( | |
<iframe | |
className={styles["artifacts-iframe"]} | |
id={frameId.current} | |
ref={ref} | |
sandbox="allow-forms allow-modals allow-scripts" | |
style={{ height }} | |
srcDoc={srcDoc} | |
onLoad={handleOnLoad} | |
/> | |
); | |
} |
export function Artifacts() { | ||
const { id } = useParams(); | ||
const [code, setCode] = useState(""); | ||
const [loading, setLoading] = useState(true); | ||
const [fileName, setFileName] = useState(""); | ||
|
||
useEffect(() => { | ||
if (id) { | ||
fetch(`${ApiPath.Artifacts}?id=${id}`) | ||
.then((res) => { | ||
if (res.status > 300) { | ||
throw Error("can not get content"); | ||
} | ||
return res; | ||
}) | ||
.then((res) => res.text()) | ||
.then(setCode) | ||
.catch((e) => { | ||
showToast(Locale.Export.Artifacts.Error); | ||
}); | ||
} | ||
}, [id]); | ||
|
||
return ( | ||
<div className={styles["artifacts"]}> | ||
<div className={styles["artifacts-header"]}> | ||
<a href={REPO_URL} target="_blank" rel="noopener noreferrer"> | ||
<IconButton bordered icon={<GithubIcon />} shadow /> | ||
</a> | ||
<div className={styles["artifacts-title"]}>NextChat Artifacts</div> | ||
<ArtifactsShareButton | ||
id={id} | ||
getCode={() => code} | ||
fileName={fileName} | ||
/> | ||
</div> | ||
<div className={styles["artifacts-content"]}> | ||
{loading && <Loading />} | ||
{code && ( | ||
<HTMLPreview | ||
code={code} | ||
autoHeight={false} | ||
height={"100%"} | ||
onLoad={(title) => { | ||
setFileName(title as string); | ||
setLoading(false); | ||
}} | ||
/> | ||
)} | ||
</div> | ||
</div> | ||
); | ||
} |
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.
Improve error handling and use useCallback
.
- Improve error handling in the
useEffect
hook by providing more specific error messages. - Use
useCallback
to memoize thesetCode
function.
+ import { useCallback } from "react";
- useEffect(() => {
+ useEffect(() => {
if (id) {
fetch(`${ApiPath.Artifacts}?id=${id}`)
.then((res) => {
if (res.status > 300) {
throw Error("Cannot get content");
}
return res;
})
.then((res) => res.text())
.then(setCode)
.catch((e) => {
showToast(Locale.Export.Artifacts.Error);
+ console.error("Fetch error:", e);
});
}
- }, [id]);
+ }, [id, setCode]);
+ const memoizedSetCode = useCallback((code: string) => {
+ setCode(code);
+ }, [setCode]);
Committable suggestion was skipped due to low confidence.
export function ArtifactsShareButton({ | ||
getCode, | ||
id, | ||
style, | ||
fileName, | ||
}: { | ||
getCode: () => string; | ||
id?: string; | ||
style?: any; | ||
fileName?: string; | ||
}) { | ||
const [loading, setLoading] = useState(false); | ||
const [name, setName] = useState(id); | ||
const [show, setShow] = useState(false); | ||
const shareUrl = useMemo( | ||
() => [location.origin, "#", Path.Artifacts, "/", name].join(""), | ||
[name], | ||
); | ||
const upload = (code: string) => | ||
id | ||
? Promise.resolve({ id }) | ||
: fetch(ApiPath.Artifacts, { | ||
method: "POST", | ||
body: code, | ||
}) | ||
.then((res) => res.json()) | ||
.then(({ id }) => { | ||
if (id) { | ||
return { id }; | ||
} | ||
throw Error(); | ||
}) | ||
.catch((e) => { | ||
showToast(Locale.Export.Artifacts.Error); | ||
}); | ||
return ( | ||
<> | ||
<div className="window-action-button" style={style}> | ||
<IconButton | ||
icon={loading ? <LoadingButtonIcon /> : <ExportIcon />} | ||
bordered | ||
title={Locale.Export.Artifacts.Title} | ||
onClick={() => { | ||
if (loading) return; | ||
setLoading(true); | ||
upload(getCode()) | ||
.then((res) => { | ||
if (res?.id) { | ||
setShow(true); | ||
setName(res?.id); | ||
} | ||
}) | ||
.finally(() => setLoading(false)); | ||
}} | ||
/> | ||
</div> | ||
{show && ( | ||
<div className="modal-mask"> | ||
<Modal | ||
title={Locale.Export.Artifacts.Title} | ||
onClose={() => setShow(false)} | ||
actions={[ | ||
<IconButton | ||
key="download" | ||
icon={<DownloadIcon />} | ||
bordered | ||
text={Locale.Export.Download} | ||
onClick={() => { | ||
downloadAs(getCode(), `${fileName || name}.html`).then(() => | ||
setShow(false), | ||
); | ||
}} | ||
/>, | ||
<IconButton | ||
key="copy" | ||
icon={<CopyIcon />} | ||
bordered | ||
text={Locale.Chat.Actions.Copy} | ||
onClick={() => { | ||
copyToClipboard(shareUrl).then(() => setShow(false)); | ||
}} | ||
/>, | ||
]} | ||
> | ||
<div> | ||
<a target="_blank" href={shareUrl}> | ||
{shareUrl} | ||
</a> | ||
</div> | ||
</Modal> | ||
</div> | ||
)} | ||
</> | ||
); |
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.
Improve error handling and use useCallback
.
- Improve error handling in the
upload
function by providing more specific error messages. - Use
useCallback
to memoize theupload
function.
+ import { useCallback } from "react";
- const upload = (code: string) =>
+ const upload = useCallback((code: string) => {
id
? Promise.resolve({ id })
: fetch(ApiPath.Artifacts, {
method: "POST",
body: code,
})
.then((res) => res.json())
.then(({ id }) => {
if (id) {
return { id };
}
throw Error("Failed to upload code");
})
.catch((e) => {
showToast(Locale.Export.Artifacts.Error);
+ console.error("Upload error:", e);
});
+ }, [id]);
Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export function ArtifactsShareButton({ | |
getCode, | |
id, | |
style, | |
fileName, | |
}: { | |
getCode: () => string; | |
id?: string; | |
style?: any; | |
fileName?: string; | |
}) { | |
const [loading, setLoading] = useState(false); | |
const [name, setName] = useState(id); | |
const [show, setShow] = useState(false); | |
const shareUrl = useMemo( | |
() => [location.origin, "#", Path.Artifacts, "/", name].join(""), | |
[name], | |
); | |
const upload = (code: string) => | |
id | |
? Promise.resolve({ id }) | |
: fetch(ApiPath.Artifacts, { | |
method: "POST", | |
body: code, | |
}) | |
.then((res) => res.json()) | |
.then(({ id }) => { | |
if (id) { | |
return { id }; | |
} | |
throw Error(); | |
}) | |
.catch((e) => { | |
showToast(Locale.Export.Artifacts.Error); | |
}); | |
return ( | |
<> | |
<div className="window-action-button" style={style}> | |
<IconButton | |
icon={loading ? <LoadingButtonIcon /> : <ExportIcon />} | |
bordered | |
title={Locale.Export.Artifacts.Title} | |
onClick={() => { | |
if (loading) return; | |
setLoading(true); | |
upload(getCode()) | |
.then((res) => { | |
if (res?.id) { | |
setShow(true); | |
setName(res?.id); | |
} | |
}) | |
.finally(() => setLoading(false)); | |
}} | |
/> | |
</div> | |
{show && ( | |
<div className="modal-mask"> | |
<Modal | |
title={Locale.Export.Artifacts.Title} | |
onClose={() => setShow(false)} | |
actions={[ | |
<IconButton | |
key="download" | |
icon={<DownloadIcon />} | |
bordered | |
text={Locale.Export.Download} | |
onClick={() => { | |
downloadAs(getCode(), `${fileName || name}.html`).then(() => | |
setShow(false), | |
); | |
}} | |
/>, | |
<IconButton | |
key="copy" | |
icon={<CopyIcon />} | |
bordered | |
text={Locale.Chat.Actions.Copy} | |
onClick={() => { | |
copyToClipboard(shareUrl).then(() => setShow(false)); | |
}} | |
/>, | |
]} | |
> | |
<div> | |
<a target="_blank" href={shareUrl}> | |
{shareUrl} | |
</a> | |
</div> | |
</Modal> | |
</div> | |
)} | |
</> | |
); | |
import { useCallback } from "react"; | |
export function ArtifactsShareButton({ | |
getCode, | |
id, | |
style, | |
fileName, | |
}: { | |
getCode: () => string; | |
id?: string; | |
style?: any; | |
fileName?: string; | |
}) { | |
const [loading, setLoading] = useState(false); | |
const [name, setName] = useState(id); | |
const [show, setShow] = useState(false); | |
const shareUrl = useMemo( | |
() => [location.origin, "#", Path.Artifacts, "/", name].join(""), | |
[name], | |
); | |
const upload = useCallback((code: string) => | |
id | |
? Promise.resolve({ id }) | |
: fetch(ApiPath.Artifacts, { | |
method: "POST", | |
body: code, | |
}) | |
.then((res) => res.json()) | |
.then(({ id }) => { | |
if (id) { | |
return { id }; | |
} | |
throw Error("Failed to upload code"); | |
}) | |
.catch((e) => { | |
showToast(Locale.Export.Artifacts.Error); | |
console.error("Upload error:", e); | |
}); | |
}, [id]); | |
return ( | |
<> | |
<div className="window-action-button" style={style}> | |
<IconButton | |
icon={loading ? <LoadingButtonIcon /> : <ExportIcon />} | |
bordered | |
title={Locale.Export.Artifacts.Title} | |
onClick={() => { | |
if (loading) return; | |
setLoading(true); | |
upload(getCode()) | |
.then((res) => { | |
if (res?.id) { | |
setShow(true); | |
setName(res?.id); | |
} | |
}) | |
.finally(() => setLoading(false)); | |
}} | |
/> | |
</div> | |
{show && ( | |
<div className="modal-mask"> | |
<Modal | |
title={Locale.Export.Artifacts.Title} | |
onClose={() => setShow(false)} | |
actions={[ | |
<IconButton | |
key="download" | |
icon={<DownloadIcon />} | |
bordered | |
text={Locale.Export.Download} | |
onClick={() => { | |
downloadAs(getCode(), `${fileName || name}.html`).then(() => | |
setShow(false), | |
); | |
}} | |
/>, | |
<IconButton | |
key="copy" | |
icon={<CopyIcon />} | |
bordered | |
text={Locale.Chat.Actions.Copy} | |
onClick={() => { | |
copyToClipboard(shareUrl).then(() => setShow(false)); | |
}} | |
/>, | |
]} | |
> | |
<div> | |
<a target="_blank" href={shareUrl}> | |
{shareUrl} | |
</a> | |
</div> | |
</Modal> | |
</div> | |
)} | |
</> | |
); |
π» εζ΄η±»ε | Change Type
π εζ΄θ―΄ζ | Description of Change
π θ‘₯ε δΏ‘ζ― | Additional Information
Summary by CodeRabbit
New Features
Bug Fixes