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.

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

Checklist Failure (type="checkbox\") #23

Closed
4 tasks done
AlbinoGeek opened this issue Aug 31, 2021 · 9 comments
Closed
4 tasks done

Checklist Failure (type="checkbox\") #23

AlbinoGeek opened this issue Aug 31, 2021 · 9 comments
Labels
💪 phase/solved Post is done

Comments

@AlbinoGeek
Copy link

Initial checklist

Affected packages and versions

rehype-format: ^4.0.0, rehype-raw: ^6.1.0, rehype-stringify: ^9.0.2, remark-gfm: ^2.0.0, remark-parse: ^10.0.0, remark-rehype: ^9.0.0, unified: ^10.1.0

Link to runnable example

No response

Steps to reproduce

Input Markdown

- [x] Some Text

Library Method

import rehypeFormat from 'rehype-format'
import rehypeRaw from 'rehype-raw'
import rehypeStringify from 'rehype-stringify'
import remarkGfm from 'remark-gfm'
import remarkParse from 'remark-parse'
import remarkRehype from 'remark-rehype'
import { unified } from 'unified'

async function render(
  markdown: string
): Promise<string> {
  const result = await unified()
    .use(remarkParse)
    .use(remarkGfm)
    .use(remarkRehype, { allowDangerousHtml: true })
    .use(rehypeRaw)
    .use(rehypeFormat)
    .use(rehypeStringify, {
      quoteSmart:       true,
      closeSelfClosing: true,
      omitOptionalTags: true,
      entities:         { useShortestReferences: true }
    })
    .process(markdown)

  return result.toString()
}

export default render
import fs from 'fs'
import { join } from 'path'

const docsDirectory = join(process.cwd(), 'docs')

export function getRawDocBySlug(slug: string): [string, string] {
  const realSlug = slug.replace(/\.mdx$/, '')
  const fullPath = join(docsDirectory, `${realSlug}.mdx`)
  const fileContents = fs.readFileSync(fullPath, 'utf8')

  return [realSlug, fileContents]
}

Usage

const DangerousMarkdown: FC<{ params: { slug: string } }> = ({ params }) => {
  const [slug, content] = getRawDocBySlug(params.slug)
  return <>
    <h1>{slug}</h1>
    <article dangerouslySetInnerHTML={{ __html: content }} />
  </>
}

Expected behavior

<h1>foo</h1>
<article>
<ul class="contains-task-list">
  <li class="task-list-item">
    <input checked="" disabled="" type="checkbox"> Some Text
  </li>
</ul>
</article>

Actual behavior

<h1>foo</h1>
<article>
<ul class="contains-task-list">
  <li class="task-list-item">
    <input checked="" disabled="" type="checkbox/"> Some Text
  </li>
</ul>
</article>

Runtime

Node v16

Package manager

yarn v2

OS

Linux

Build and bundle tools

Next.js

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Aug 31, 2021
@AlbinoGeek
Copy link
Author

It may have been sublte, the issue is it's rendering type="checkbox\" instead of type="checkbox"

@wooorm
Copy link
Member

wooorm commented Aug 31, 2021

It looks like render is not used, and that you’re mixing MDX files with non-MDX-enabled remark?

@ChristianMurphy
Copy link
Member

I'd second @wooorm's question,
And add that testing the example you provided, modifying it to call render https://codesandbox.io/s/dangerous-html-example-ff3zl doesn't appear to have the issue you described.
Also since you are using react, it may be worth considering using https://github.com/rehypejs/rehype-react or https://github.com/wooorm/xdm#next to render directly to React in Next.

@AlbinoGeek
Copy link
Author

I'd second @wooorm's question,
And add that testing the example you provided, modifying it to call render https://codesandbox.io/s/dangerous-html-example-ff3zl doesn't appear to have the issue you described.
Also since you are using react, it may be worth considering using https://github.com/rehypejs/rehype-react or https://github.com/wooorm/xdm#next to render directly to React in Next.

Unfortunately none of those methods allow for the use of all of the following:

  • Allow mixed input (HTML and Markdown)
  • Meta (Search Engine Optimization)
  • Server-Side Generation (Pages)
  • Partials (Server-Side Includes)
  • Client-Side Rendering (Editor Previews)

For this reason, the site has two different methods to render Markdown:

  1. From the filesystem on the server in getStaticProps more or less as you showed in your sandbox
  2. From an AST tree parsed on the server and rendered in the browser via a MarkdownRenderer component

Method 1

Basically your codesandbox code, except render's usage is really:

import MDXLayout from '../components/MDXLayout'
import { getAllDocs, getDocBySlug } from '../lib/docs'
import render from '../lib/markdown'

type Props = {
  meta: any
  content: string
}

const Doc = ({ meta,  content }: Props) => {
  return (
    <MDXLayout meta={meta}>
      {content}
    </MDXLayout>
  )
}

export async function getStaticProps({ params }: { params: { slug: string } }) {
  const doc = getDocBySlug(params.slug)
  const content = await render(doc.content || '')

  return {
    props: {
      ...doc,
      content
    }
  }
}

export async function getStaticPaths() {
  const docs = getAllDocs()

  const allPaths: { params: any }[] = []
  docs.forEach((doc) => {
    allPaths.push({
      params: {
        slug: doc.slug
      }
    })
  })

  return {
    paths:    allPaths,
    fallback: true
  }
}

export default Doc
import type { FC } from 'react'
import BaseLayout from './BaseLayout'

const MDXLayout: FC<{ children: string; menu: string; meta: any }> = ({
  children,
  meta: pageMeta
}) => {
  const meta = {
    title:       'Wiki',
    description: '',
    cardImage:   '',
    ...pageMeta
  }

  return (
    <BaseLayout
      meta={meta}
    >
      <article dangerouslySetInnerHTML={{ __html: children }} />
    </BaseLayout>
  )
}

export default MDXLayout
import { useRouter } from 'next/dist/client/router'
import Head from 'next/head'
import Link from 'next/link'
import { FC } from 'react'

const BaseLayout: FC<{ menu?: JSX.Element; meta: any }> = ({
  children,
  meta
}) => {
  const router = useRouter()

  return (
    <>
      <Head>
        <title>{meta.title}</title>
      </Head>
      <div className="page">
        <h1 style={{ paddingTop: 0 }}>{meta.title}</h1>

        {children}
      </div>
    </>
  )

Method 2

Specifically following this guide: https://css-tricks.com/responsible-markdown-in-next-js/

I have (though messy) dragged all the required code snippets out of my project into the codesandbox:

https://codesandbox.io/s/dangerous-html-example-forked-zz7yl?file=/src/App.js


Unfortunately I cannot reproduce this issue using this method either, which is beyond frustrating.

Well, I couldn't figure out how to codesandbox a next app to test the server-side include, but the client-side method is shown above.

If there's nothing obvious about these methods described above that's broken, I'll have to publish the app itself as minimal reproduction, which.. eww, I don't want you guys to have to sift through that.

@AlbinoGeek
Copy link
Author

AlbinoGeek commented Aug 31, 2021

I found the culprit:

Replace rehypeFormat with rehypePresetMinify (in any of the examples) and 💥

This might be an issue for them instead? (should I move this?)
https://github.com/rehypejs/rehype-minify

@ChristianMurphy
Copy link
Member

Unfortunately none of those methods allow for the use of all of the following:

  • Allow mixed input (HTML and Markdown)
  • Meta (Search Engine Optimization)
  • Server-Side Generation (Pages)
  • Partials (Server-Side Includes)
  • Client-Side Rendering (Editor Previews)

Both xdm and rehype-react allow for all of the above.
To clarify since there seems to be some confusion, these would replace remark-parse and/or rehype-stringify specifically, most of the rest of the unified pipeline would remain the same.
You'd just get an easier to use/safer output from the pipeline.

Unfortunately I cannot reproduce this issue using this method either, which is beyond frustrating.

Could you share the versions of dependencies you are using locally?
It could be you are using older versions of dependencies which could in theory have a bug that has already been patched in latest.
Or version incompatibility between plugins causing an issue.

Well, I couldn't figure out how to codesandbox a next app to test the server-side include, but the client-side method is shown above.

Code Sandbox has a Next starter which may help https://codesandbox.io/s/admiring-leftpad-iywjj (or click "Create Sandbox", and search for "Next" in the template list)

@AlbinoGeek
Copy link
Author

AlbinoGeek commented Aug 31, 2021

@ChristianMurphy Sorry about that, please see the message just above where I finally found the issue.

Also, here are the package versions I am using (should all be latest):

  "dependencies": {
    "@babel/core": "^7.15.0",
    "@jsdevtools/rehype-toc": "^3.0.2",
    "gray-matter": "^4.0.3",
    "next": "^11.1.2",
    "react": "^17.0.2",
    "react-dom": "^17.0.2",
    "react-is": "^17.0.2",
    "rehype-format": "^4.0.0",
    "rehype-partials": "^1.0.6",
    "rehype-preset-minify": "^6.0.0",
    "rehype-raw": "^6.1.0",
    "rehype-react": "^7.0.1",
    "rehype-slug": "^5.0.0",
    "rehype-stringify": "^9.0.2",
    "remark-emoji": "^3.0.1",
    "remark-footnotes": "^4.0.1",
    "remark-gfm": "^2.0.0",
    "remark-parse": "^10.0.0",
    "remark-rehype": "^9.0.0",
    "remark-wiki-link": "^1.0.4",
    "styled-components": "^5.3.1",
    "unified": "^10.1.0"
  },

  "devDependencies": {
    "@types/react": "^17.0.19",
    "@types/styled-components": "^5.1.13",
    "@typescript-eslint/eslint-plugin": "^4.30.0",
    "@typescript-eslint/parser": "^4.30.0",
    "babel-plugin-styled-components": "^1.13.2",
    "eslint": "^7.32.0",
    "eslint-config-next": "^11.1.2",
    "eslint-config-prettier": "^8.3.0",
    "eslint-plugin-react": "^7.25.1",
    "prettier": "^2.3.2",
    "prettier-eslint": "^13.0.0",
    "typescript": "^4.4.2"
  }

Specifically:

https://github.com/rehypejs/rehype-minify/tree/main/packages/rehype-preset-minify

@ChristianMurphy
Copy link
Member

ChristianMurphy commented Aug 31, 2021

Replace rehypeFormat with rehypePresetMinify (in any of the examples) and 💥

This might be an issue for them instead? (should I move this?)
https://github.com/rehypejs/rehype-minify

I'm not sure it is.
With that additional context, I updated the example from before to try to reproduce the issue https://codesandbox.io/s/rehype-minify-7x65y
Try commenting and uncommenting line 16

      closeSelfClosing: true

it switches between a checkbox and text box, it seems like it may be related to the closeSelfClosing option 🤔

@wooorm
Copy link
Member

wooorm commented Aug 31, 2021

Why are you setting closeSelfClosing? It’s a useless option for HTML, and it only makes sense when using XML/MDX. But, you are missing XDM functionality in your pipeline?

It’s a bug in hast-util-to-html, but still, this bug is a manifestation of another problem.

wooorm added a commit to syntax-tree/hast-util-to-html that referenced this issue Aug 31, 2021
@wooorm wooorm added the 💪 phase/solved Post is done label Aug 31, 2021
@github-actions github-actions bot removed the 🤞 phase/open Post is being triaged manually label Aug 31, 2021
@wooorm wooorm closed this as completed Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 phase/solved Post is done
Development

No branches or pull requests

3 participants