fix(prose): update typing for `Link` #360

seaerchin · 2024-07-24T11:22:02Z

Problem

We need to allow users to set links inside our tiptap component. However, tiptap sets teh href attribute inside the attrs object whereas our validator expects a top level href attribute. this causes any link insertion to break on validation.

Solution

Update the LinkMarkSchema to be Unsafe to avoid deeply nested types
Update the LinkMarkSchema to have a href attribute so that validation can pass

vercel · 2024-07-24T11:22:06Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
isomer-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 29, 2024 4:53am

seaerchin · 2024-07-24T11:33:31Z

tooling/build/scripts/generate-sitemap.js

+                      attrs: {
+                        href: child.permalink,
+                      },


only relevant change because we update the href to be inside attrs now. i searched the 2 .js files in this directory to check if we needed to update href

fact-check me @dcshzj if this is enoguh

add to tt extension

add detailed link schema with ref to tiptap

use unsafe

add nullable

update generation

dcshzj · 2024-07-25T01:53:55Z

packages/components/src/interfaces/native/Text.ts

+        target: Type.Union([
+          // NOTE: Taken with reference to
+          // https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-target
+          // There's technically 1 more property - unfencedTop but we're disallowing
+          // because it allows embedded frames to traverse beyond its root
+          Type.Literal("_blank"),
+          Type.Literal("_self"),
+          Type.Literal("_parent"),
+          Type.Literal("_top"),
+        ]),
+        // NOTE: The href given by tiptap here
+        // https://github.com/ueberdosis/tiptap/blob/main/packages/extension-link/src/link.ts
+        // defaults to `null` href but href could also be passed as `undefined`
+        href: Type.Union([Type.String(), Type.Null()]),
+        rel: Type.String(),
+        class: Type.Union([Type.String(), Type.Null()]),


issue: I don't think we should match the schema that Tiptap provides entirely, our schema is intended to be a subset of the schema because we only want to allow certain behaviours instead of all the available options that HTML/Tiptap provides. Adding all here means we lose the signal on what is allowed and what isn't.

In general we override the rel and class when we output to the user, so those properties will be ignored. For target we should only support _blank.

done - where do i see the properties that we strip? couldn't find a Text component

Mainly in getSanitizedInlineContent.ts:

isomer/packages/components/src/utils/getSanitizedInlineContent.ts

Lines 5 to 42 in c19c5d5

export const getSanitizedInlineContent = (content: string) => {

DOMPurify.addHook("beforeSanitizeElements", (curr) => {

// Add rel="noopener noreferrer nofollow" to all anchor tags and

// open them in a new tab, if they point to an external site

if (curr.tagName !== "A") {

return curr

}

const href = curr.getAttribute("href")

if (href && (href.startsWith("http://") || href.startsWith("https://"))) {

curr.setAttribute("rel", "noopener noreferrer nofollow")

curr.setAttribute("target", "_blank")

}

return curr

})

const sanitizedContent = DOMPurify.sanitize(content, {

ALLOWED_TAGS: [

"b", // bold

"strong", // bold

"i", // italic

"em", // italic

"u", // underline

"s", // strikethrough

"del", // strikethrough

"strike", // strikethrough

"sub", // subscript

"sup", // superscript

"a", // anchor

"br", // line break

"code", // code

],

ALLOWED_ATTR: ["href", "rel", "target"],

})

return sanitizedContent

}

dcshzj · 2024-07-26T02:32:53Z

packages/components/src/interfaces/native/Text.ts

  {
    type: Type.Literal("link", { default: "link" }),
-    href: Type.String(),
+    attrs: Type.Object({
+      target: Type.Literal("_blank"),


issue: hmm this property should be optional

Suggested change

target: Type.Literal("_blank"),

target: Type.Optional(Type.Literal("_blank")),

i don't think that's quite right because we're using tiptap's link schema. they default it to the options.target, which is _blank by default, see ss

Hmm but we are not using Tiptap's link schema entirely. In HTML it is entirely possible to omit the target attribute and the link will have a default behaviour of _self. This schema means that all links must have _blank, which may not be the case for internal links.

you're right! i misunderstood the schema here because i thought it was only validating tiptap. i'll set it to optional("_self" | "_blank") wdyt?

seaerchin requested a review from a team as a code owner July 24, 2024 11:22

seaerchin temporarily deployed to staging July 24, 2024 11:22 — with GitHub Actions Inactive

seaerchin temporarily deployed to staging July 24, 2024 11:29 — with GitHub Actions Inactive

seaerchin force-pushed the fix/prose-link branch from 78ae23f to 7bdf5d8 Compare July 24, 2024 11:30

seaerchin temporarily deployed to staging July 24, 2024 11:30 — with GitHub Actions Inactive

seaerchin temporarily deployed to staging July 24, 2024 11:32 — with GitHub Actions Inactive

seaerchin commented Jul 24, 2024

View reviewed changes

vercel bot deployed to Preview July 24, 2024 11:36 View deployment

Base automatically changed from feat/prose-renderer to main July 24, 2024 11:48

seaerchin added 8 commits July 24, 2024 23:57

Merge branch 'main' into feat/prose-renderer

5eb33a5

Merge branch 'main' into feat/prose-renderer

abdd008

fix: tiptap editor

7fa6506

add to tt extension

feat: text

a683f0c

add detailed link schema with ref to tiptap

chore: fix stories

cbba925

fix: text

634473a

use unsafe

fix: class

79b920b

add nullable

fix: generate sitemap

330c7f8

update generation

seaerchin force-pushed the fix/prose-link branch from 15f1ee4 to 330c7f8 Compare July 24, 2024 15:57

seaerchin temporarily deployed to staging July 24, 2024 15:57 — with GitHub Actions Inactive

vercel bot deployed to Preview July 24, 2024 16:00 View deployment

dcshzj reviewed Jul 25, 2024

View reviewed changes

seaerchin temporarily deployed to staging July 25, 2024 05:59 — with GitHub Actions Inactive

chore: remove extra props

f2c7070

refactor: reference the schema rather than defining types

812972c

seaerchin force-pushed the fix/prose-link branch from 838adf5 to 812972c Compare July 25, 2024 06:01

seaerchin temporarily deployed to staging July 25, 2024 06:02 — with GitHub Actions Inactive

seaerchin requested a review from dcshzj July 25, 2024 06:02

vercel bot deployed to Preview July 25, 2024 06:05 View deployment

seaerchin requested a review from a team July 25, 2024 06:42

dcshzj reviewed Jul 26, 2024

View reviewed changes

seaerchin requested review from dcshzj and a team July 26, 2024 05:49

fix: update link target

bb5be6a

seaerchin temporarily deployed to staging July 29, 2024 04:50 — with GitHub Actions Inactive

vercel bot deployed to Preview July 29, 2024 04:53 View deployment

dcshzj approved these changes Jul 29, 2024

View reviewed changes

seaerchin merged commit 98fbbde into main Jul 29, 2024
17 of 19 checks passed

seaerchin deleted the fix/prose-link branch July 29, 2024 06:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(prose): update typing for `Link` #360

fix(prose): update typing for `Link` #360

seaerchin commented Jul 24, 2024

vercel bot commented Jul 24, 2024 •

edited

Loading

seaerchin Jul 24, 2024

dcshzj Jul 25, 2024

seaerchin Jul 25, 2024

dcshzj Jul 25, 2024

dcshzj Jul 26, 2024

seaerchin Jul 26, 2024

dcshzj Jul 29, 2024

seaerchin Jul 29, 2024

	export const getSanitizedInlineContent = (content: string) => {
	DOMPurify.addHook("beforeSanitizeElements", (curr) => {
	// Add rel="noopener noreferrer nofollow" to all anchor tags and
	// open them in a new tab, if they point to an external site
	if (curr.tagName !== "A") {
	return curr
	}

	const href = curr.getAttribute("href")
	if (href && (href.startsWith("http://") \|\| href.startsWith("https://"))) {
	curr.setAttribute("rel", "noopener noreferrer nofollow")
	curr.setAttribute("target", "_blank")
	}

	return curr
	})

	const sanitizedContent = DOMPurify.sanitize(content, {
	ALLOWED_TAGS: [
	"b", // bold
	"strong", // bold
	"i", // italic
	"em", // italic
	"u", // underline
	"s", // strikethrough
	"del", // strikethrough
	"strike", // strikethrough
	"sub", // subscript
	"sup", // superscript
	"a", // anchor
	"br", // line break
	"code", // code
	],
	ALLOWED_ATTR: ["href", "rel", "target"],
	})

	return sanitizedContent
	}

	target: Type.Literal("_blank"),
	target: Type.Optional(Type.Literal("_blank")),

fix(prose): update typing for Link #360

fix(prose): update typing for Link #360

Conversation

seaerchin commented Jul 24, 2024

Problem

Solution

vercel bot commented Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fix(prose): update typing for `Link` #360

fix(prose): update typing for `Link` #360

vercel bot commented Jul 24, 2024 •

edited

Loading