-
Notifications
You must be signed in to change notification settings - Fork 3.1k
[WIKI-575] fix: add unique keys to add nodeview wrappers #8209
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
base: preview
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| import type { TBlockNodeBaseAttributes } from "../unique-id/types"; | ||
|
|
||
| export type TCodeBlockAttributes = TBlockNodeBaseAttributes & { | ||
| language: string | null; | ||
| }; |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,5 +1,7 @@ | ||||||
| import type { NodeViewProps } from "@tiptap/react"; | ||||||
| import { NodeViewWrapper } from "@tiptap/react"; | ||||||
| import { useMemo } from "react"; | ||||||
| import { v4 as uuidv4 } from "uuid"; | ||||||
|
Comment on lines
+3
to
+4
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. Remove unused imports. The imports Apply this diff: -import { useMemo } from "react";
-import { v4 as uuidv4 } from "uuid";📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| // extension config | ||||||
| import type { TMentionExtensionOptions } from "./extension-config"; | ||||||
| // extension types | ||||||
|
|
@@ -19,7 +21,7 @@ export function MentionNodeView(props: MentionNodeViewProps) { | |||||
| } = props; | ||||||
|
|
||||||
| return ( | ||||||
| <NodeViewWrapper className="mention-component inline w-fit"> | ||||||
| <NodeViewWrapper className="mention-component inline w-fit" key={`mention-${attrs.id}`}> | ||||||
| {(extension.options as TMentionExtensionOptions).renderComponent({ | ||||||
| entity_identifier: attrs[EMentionComponentAttributeNames.ENTITY_IDENTIFIER] ?? "", | ||||||
| entity_name: attrs[EMentionComponentAttributeNames.ENTITY_NAME] ?? "user_mention", | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| /** | ||
| * Base attributes for all block nodes that have the unique-id extension. | ||
| * All block node attribute types should extend this. | ||
| */ | ||
| export interface TBlockNodeBaseAttributes { | ||
| id?: string | null; | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,7 @@ import { ReactNodeViewRenderer, NodeViewWrapper } from "@tiptap/react"; | |||||||||||||||||||||||||||||||||
| import type { NodeViewProps } from "@tiptap/react"; | ||||||||||||||||||||||||||||||||||
| // local imports | ||||||||||||||||||||||||||||||||||
| import { WorkItemEmbedExtensionConfig } from "./extension-config"; | ||||||||||||||||||||||||||||||||||
| import type { TWorkItemEmbedAttributes } from "./types"; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| type Props = { | ||||||||||||||||||||||||||||||||||
| widgetCallback: ({ | ||||||||||||||||||||||||||||||||||
|
|
@@ -18,15 +19,18 @@ type Props = { | |||||||||||||||||||||||||||||||||
| export function WorkItemEmbedExtension(props: Props) { | ||||||||||||||||||||||||||||||||||
| return WorkItemEmbedExtensionConfig.extend({ | ||||||||||||||||||||||||||||||||||
| addNodeView() { | ||||||||||||||||||||||||||||||||||
| return ReactNodeViewRenderer((issueProps: NodeViewProps) => ( | ||||||||||||||||||||||||||||||||||
| <NodeViewWrapper> | ||||||||||||||||||||||||||||||||||
| {props.widgetCallback({ | ||||||||||||||||||||||||||||||||||
| issueId: issueProps.node.attrs.entity_identifier, | ||||||||||||||||||||||||||||||||||
| projectId: issueProps.node.attrs.project_identifier, | ||||||||||||||||||||||||||||||||||
| workspaceSlug: issueProps.node.attrs.workspace_identifier, | ||||||||||||||||||||||||||||||||||
| })} | ||||||||||||||||||||||||||||||||||
| </NodeViewWrapper> | ||||||||||||||||||||||||||||||||||
| )); | ||||||||||||||||||||||||||||||||||
| return ReactNodeViewRenderer((issueProps: NodeViewProps) => { | ||||||||||||||||||||||||||||||||||
| const attrs = issueProps.node.attrs as TWorkItemEmbedAttributes; | ||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||
| <NodeViewWrapper key={`work-item-embed-${attrs.id}`}> | ||||||||||||||||||||||||||||||||||
| {props.widgetCallback({ | ||||||||||||||||||||||||||||||||||
| issueId: attrs.entity_identifier!, | ||||||||||||||||||||||||||||||||||
|
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. Risky non-null assertion on optional field. The non-null assertion Consider adding a guard or early return: return ReactNodeViewRenderer((issueProps: NodeViewProps) => {
const attrs = issueProps.node.attrs as TWorkItemEmbedAttributes;
+ if (!attrs.entity_identifier) {
+ return <NodeViewWrapper key={`work-item-embed-${attrs.id}`}>Invalid work item</NodeViewWrapper>;
+ }
return (
<NodeViewWrapper key={`work-item-embed-${attrs.id}`}>
{props.widgetCallback({
- issueId: attrs.entity_identifier!,
+ issueId: attrs.entity_identifier,
projectId: attrs.project_identifier,
workspaceSlug: attrs.workspace_identifier,
})}
</NodeViewWrapper>
);
});📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| projectId: attrs.project_identifier, | ||||||||||||||||||||||||||||||||||
| workspaceSlug: attrs.workspace_identifier, | ||||||||||||||||||||||||||||||||||
| })} | ||||||||||||||||||||||||||||||||||
| </NodeViewWrapper> | ||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| import type { TBlockNodeBaseAttributes } from "../unique-id/types"; | ||
|
|
||
| export type TWorkItemEmbedAttributes = TBlockNodeBaseAttributes & { | ||
| entity_identifier: string | undefined; | ||
| project_identifier: string | undefined; | ||
| workspace_identifier: string | undefined; | ||
| entity_name: string | undefined; | ||
| }; |
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.
🧩 Analysis chain
🏁 Script executed:
Repository: makeplane/plane
Length of output: 5228
Key prop uses optional
idattribute that could be undefined—ensure initialization guarantees unique IDs.The
key={callout-block-${node.attrs.id}}at line 36 depends onidfromTBlockNodeBaseAttributes, which definesidas optional (id?: string | null). Ifidis undefined or null, the key becomescallout-block-undefinedorcallout-block-null, defeating the reconciliation objective of the PR.Verify that callout nodes are always initialized with a valid, unique ID before rendering, or add runtime checks/defaults in the key expression to ensure stability.
🤖 Prompt for AI Agents