Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(react): Fix incorrect extensionAttributes value #5588

Merged
merged 5 commits into from
Sep 24, 2024
Merged

fix(react): Fix incorrect extensionAttributes value #5588

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
963d5b7
fix(react): Fix incorrect extensionAttributes value
YaoKaiLun Sep 4, 2024
561f5d1
Merge branch 'ueberdosis:develop' into fix/react-attrs
YaoKaiLun Sep 22, 2024
a091d53
refactor(react): Moving the type filter to "getRenderedAttributes" fu…
YaoKaiLun Sep 23, 2024
f0b38d3
Merge branch 'develop' into fix/react-attrs
nperez0111 Sep 24, 2024
9ddc31b
chore: update changeset
nperez0111 Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tricky-apricots-smash.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@tiptap/react": patch
---

Fix incorrect extensionAttributes value
4 changes: 3 additions & 1 deletion packages/react/src/ReactNodeViewRenderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,9 @@ export class ReactNodeView<
let attrsObj: Record<string, string> = {}

if (typeof this.options.attrs === 'function') {
const extensionAttributes = this.editor.extensionManager.attributes
const extensionAttributes = this.editor.extensionManager.attributes.filter(
attribute => attribute.type === this.node.type.name,
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, what did you run into that warrants this change? It is not clear to me why it is needed? It feels like something that is better placed as a responsibility of getRenderedAttributes instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "attrs" function would accept all extensions' acttributes, it wasn't what i expected, it only need accpet relative extensions' acttributes. Maybe there's a better way to write it that I don't know?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, you are right here. I just think it is better implemented in the function getRenderedAttributes instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I normally would just apply my patch on top of the branch, but some reason it was disabled, so here is the patch I would apply to this PR

diff --git a/packages/core/src/helpers/getRenderedAttributes.ts b/packages/core/src/helpers/getRenderedAttributes.ts
index 5dd0855a2..fd85e699a 100644
--- a/packages/core/src/helpers/getRenderedAttributes.ts
+++ b/packages/core/src/helpers/getRenderedAttributes.ts
@@ -8,6 +8,9 @@ export function getRenderedAttributes(
   extensionAttributes: ExtensionAttribute[],
 ): Record<string, any> {
   return extensionAttributes
+    .filter(
+      attribute => attribute.type === nodeOrMark.type.name,
+    )
     .filter(item => item.attribute.rendered)
     .map(item => {
       if (!item.attribute.renderHTML) {
diff --git a/packages/react/src/ReactNodeViewRenderer.tsx b/packages/react/src/ReactNodeViewRenderer.tsx
index 054b69130..4f16c0c8c 100644
--- a/packages/react/src/ReactNodeViewRenderer.tsx
+++ b/packages/react/src/ReactNodeViewRenderer.tsx
@@ -304,9 +304,7 @@ export class ReactNodeView<
       let attrsObj: Record<string, string> = {}
 
       if (typeof this.options.attrs === 'function') {
-        const extensionAttributes = this.editor.extensionManager.attributes.filter(
-          attribute => attribute.type === this.node.type.name,
-        )
+        const extensionAttributes = this.editor.extensionManager.attributes
         const HTMLAttributes = getRenderedAttributes(this.node, extensionAttributes)
 
         attrsObj = this.options.attrs({ node: this.node, HTMLAttributes })

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's great! But i am not sure if it will break the previous features?

const HTMLAttributes = getRenderedAttributes(this.node, extensionAttributes)

attrsObj = this.options.attrs({ node: this.node, HTMLAttributes })
Expand Down