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

feat: Ensure the article appears exactly as it will when published #1102 #1153

Merged
merged 14 commits into from
Oct 24, 2024
Merged

feat: Ensure the article appears exactly as it will when published #1102 #1153

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
a059d85
feat: Ensure the article appears exactly as it will when published
dineshsutihar Oct 18, 2024
560a7f7
Update package-lock.json to fix dependency issues
dineshsutihar Oct 18, 2024
42cd56d
Merge branch 'codu-code:develop' into fix-1102-implment-wysiwyg-to-se…
dineshsutihar Oct 18, 2024
402bfd2
Merge branch 'develop' into fix-1102-implment-wysiwyg-to-see-article-…
dineshsutihar Oct 18, 2024
417cb27
Refactor JSON parsing to optimize performance by parsing 'post.body' …
dineshsutihar Oct 18, 2024
753bc7f
fix: add DOMPurify for sanitizing HTML to prevent XSS attacks
dineshsutihar Oct 18, 2024
37a91d2
fix: align pre-publish editor design to match post-publish article ap…
dineshsutihar Oct 19, 2024
917d78e
Merge branch 'develop' into fix-1102-implment-wysiwyg-to-see-article-…
dineshsutihar Oct 20, 2024
516e714
refactor: remove unnecessary divs and clean up code without affecting…
dineshsutihar Oct 21, 2024
b174cf9
Merge branch 'develop' into fix-1102-implment-wysiwyg-to-see-article-…
NiallJoeMaher Oct 21, 2024
e6dc4c5
fix: lint issue by replacing the type form any to JSONContent
dineshsutihar Oct 22, 2024
c548efd
Merge branch 'develop' into fix-1102-implment-wysiwyg-to-see-article-…
dineshsutihar Oct 22, 2024
2fc7387
Merge branch 'develop' into fix-1102-implment-wysiwyg-to-see-article-…
NiallJoeMaher Oct 22, 2024
0a49461
style: fix prettier formatting issues
dineshsutihar Oct 23, 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
52 changes: 45 additions & 7 deletions app/(app)/articles/[slug]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from "react";
import type { RenderableTreeNode } from "@markdoc/markdoc";
import Markdoc from "@markdoc/markdoc";
import Link from "next/link";
import BioBar from "@/components/BioBar/BioBar";
Expand All @@ -13,6 +14,9 @@ import ArticleAdminPanel from "@/components/ArticleAdminPanel/ArticleAdminPanel"
import { type Metadata } from "next";
import { getPost } from "@/server/lib/posts";
import { getCamelCaseFromLower } from "@/utils/utils";
import { generateHTML } from "@tiptap/html";
import { TiptapExtensions } from "@/components/editor/editor/extensions";
import DOMPurify from 'isomorphic-dompurify';

type Props = { params: { slug: string } };

Expand Down Expand Up @@ -57,6 +61,20 @@ export async function generateMetadata({ params }: Props): Promise<Metadata> {
};
}

const parseJSON = (str: string): any | null => {
try {
return JSON.parse(str);
} catch (e) {
return null;
}
};

const renderSanitizedTiptapContent = (jsonContent: JSON) => {
const rawHtml = generateHTML(jsonContent, [...TiptapExtensions]);
// Sanitize the HTML
return DOMPurify.sanitize(rawHtml);
};
dineshsutihar marked this conversation as resolved.
Show resolved Hide resolved

const ArticlePage = async ({ params }: Props) => {
const session = await getServerAuthSession();
const { slug } = params;
Expand All @@ -66,11 +84,25 @@ const ArticlePage = async ({ params }: Props) => {
const post = await getPost({ slug });

if (!post) {
notFound();
return notFound();
}

const parsedBody = parseJSON(post.body);
const isTiptapContent = parsedBody?.type === "doc";

let renderedContent: string | RenderableTreeNode;
dineshsutihar marked this conversation as resolved.
Show resolved Hide resolved

if (isTiptapContent && parsedBody) {
const jsonContent = parsedBody;
renderedContent = renderSanitizedTiptapContent(jsonContent);
} else {
const ast = Markdoc.parse(post.body);
const transformedContent = Markdoc.transform(ast, config);
renderedContent = Markdoc.renderers.react(transformedContent, React, {
components: markdocComponents,
}) as unknown as string;
Comment on lines +91 to +104
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

LGTM: Improved content type detection and rendering

The new logic effectively detects and renders both Tiptap and Markdoc content, aligning with the PR objectives.

Remove the unnecessary type assertion in the Markdoc rendering path:

- renderedContent = Markdoc.renderers.react(transformedContent, React, {
-   components: markdocComponents,
- }) as unknown as string;
+ renderedContent = Markdoc.renderers.react(transformedContent, React, {
+   components: markdocComponents,
+ });

This change eliminates the incorrect casting of a React element to string, improving type safety.

📝 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.

Suggested change
const parsedBody = parseJSON(post.body);
const isTiptapContent = parsedBody?.type === "doc";
let renderedContent: string | RenderableTreeNode;
if (isTiptapContent && parsedBody) {
const jsonContent = parsedBody;
renderedContent = renderSanitizedTiptapContent(jsonContent);
} else {
const ast = Markdoc.parse(post.body);
const transformedContent = Markdoc.transform(ast, config);
renderedContent = Markdoc.renderers.react(transformedContent, React, {
components: markdocComponents,
}) as unknown as string;
const parsedBody = parseJSON(post.body);
const isTiptapContent = parsedBody?.type === "doc";
let renderedContent: string | RenderableTreeNode;
if (isTiptapContent && parsedBody) {
const jsonContent = parsedBody;
renderedContent = renderSanitizedTiptapContent(jsonContent);
} else {
const ast = Markdoc.parse(post.body);
const transformedContent = Markdoc.transform(ast, config);
renderedContent = Markdoc.renderers.react(transformedContent, React, {
components: markdocComponents,
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keep the type assertion because renderedContent needs to be treated as a string for use in dangerouslySetInnerHTML, and removing it could lead to runtime issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dineshsutihar, understood. Thanks for the clarification. Keeping the type assertion ensures correct behavior when using dangerouslySetInnerHTML.

🐰🤔

✏️ Learnings added 
Learnt from: dineshsutihar
PR: codu-code/codu#1153
File: app/(app)/articles/[slug]/page.tsx:91-104
Timestamp: 2024-10-22T08:43:13.045Z
Learning: In `app/(app)/articles/[slug]/page.tsx`, when rendering content, the `renderedContent` needs to be treated as a string using a type assertion because it's used with `dangerouslySetInnerHTML`, and removing the type assertion could lead to runtime issues.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

}

const ast = Markdoc.parse(post.body);
const content = Markdoc.transform(ast, config);

return (
<>
Expand All @@ -83,10 +115,16 @@ const ArticlePage = async ({ params }: Props) => {
/>
<div className="mx-auto break-words px-2 pb-4 sm:px-4 md:max-w-3xl">
<article className="prose mx-auto max-w-3xl dark:prose-invert lg:prose-lg">
<h1>{post.title}</h1>
{Markdoc.renderers.react(content, React, {
components: markdocComponents,
})}
{!isTiptapContent && (<h1>{post.title}</h1>)}

{isTiptapContent ? (
<div
dangerouslySetInnerHTML={{ __html: renderedContent }}
dineshsutihar marked this conversation as resolved.
Show resolved Hide resolved
className="tiptap-content"
/>
) : (
<div>{renderedContent}</div>
)}
</article>
{post.tags.length > 0 && (
<section className="flex flex-wrap gap-3">
Expand Down
3 changes: 2 additions & 1 deletion components/editor/editor/RenderPost.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from "react";
import { TiptapExtensions } from "./extensions";
import { EditorContent, useEditor } from "@tiptap/react";
import SlashCommand from "./extensions/slash-command";

interface RenderPostProps {
json: string;
Expand All @@ -11,7 +12,7 @@ const RenderPost = ({ json }: RenderPostProps) => {

const editor = useEditor({
editable: false,
extensions: [...TiptapExtensions],
extensions: [...TiptapExtensions, SlashCommand],
content,
});

Expand Down
1 change: 0 additions & 1 deletion components/editor/editor/extensions/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ export const TiptapExtensions = [
return "type / to see a list of formatting features";
},
}),
SlashCommand,
TextStyle,
Link.configure({
HTMLAttributes: {
Expand Down
Loading
Loading