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

Add more options to playground, directives, format, etc #2295

Merged
merged 15 commits into from
Jun 10, 2023
Merged

Add more options to playground, directives, format, etc #2295

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
d92eed1
add support for remark-directive + format md
slorber May 4, 2023
a4a194d
Add ability to remove ast positions by default
slorber May 4, 2023
ef61762
Update docs/_component/editor.client.js
slorber May 10, 2023
ca34275
Use radio for format option
slorber May 10, 2023
8142872
Add ability to remove esast positional infos
slorber May 10, 2023
d478105
import order
slorber May 10, 2023
748281f
Merge branch 'main' into slorber/playground-format-md
slorber May 10, 2023
07a1ddb
Add rehype-raw option
slorber May 11, 2023
62ea85d
remove useless export
slorber Jun 9, 2023
bbc84f5
Update docs/_component/editor.client.js
slorber Jun 9, 2023
68d707d
Merge branch 'slorber/playground-format-md' of github.com:slorber/mdx…
slorber Jun 9, 2023
3e1b89e
Merge branch 'main' into slorber/playground-format-md
slorber Jun 9, 2023
90409dc
use clear range for remark-directive
slorber Jun 9, 2023
8a1b0ff
add rehypeRaw passThrough
slorber Jun 9, 2023
cc12b73
move removePositionEsast at the bottom of the file
slorber Jun 9, 2023
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
55 changes: 54 additions & 1 deletion docs/_component/editor.client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import {reporter} from 'vfile-reporter'
import {evaluate} from '@mdx-js/mdx'
import remarkGfm from 'remark-gfm'
import remarkFrontmatter from 'remark-frontmatter'
import remarkDirective from 'remark-directive'
import remarkMath from 'remark-math'
import {removePosition} from 'unist-util-remove-position'
import CodeMirror from 'rodemirror'
import {basicSetup} from 'codemirror'
import {markdown as langMarkdown} from '@codemirror/lang-markdown'
Expand All @@ -29,7 +31,8 @@ function useMdx(defaults) {
const [state, setState] = useState({...defaults, file: null})
const {run: setConfig} = useDebounceFn(
async (config) => {
const file = new VFile({basename: 'example.mdx', value: config.value})
const basename = config.formatMd ? 'example.md' : 'example.mdx'
const file = new VFile({basename, value: config.value})

const capture = (name) => () => (tree) => {
file.data[name] = tree
Expand All @@ -40,6 +43,7 @@ function useMdx(defaults) {
if (config.gfm) remarkPlugins.push(remarkGfm)
if (config.frontmatter) remarkPlugins.push(remarkFrontmatter)
if (config.math) remarkPlugins.push(remarkMath)
if (config.directive) remarkPlugins.push(remarkDirective)

remarkPlugins.push(capture('mdast'))

Expand All @@ -53,6 +57,12 @@ function useMdx(defaults) {
recmaPlugins: [capture('esast')]
})
).default

if (!config.position) {
removePosition(file.data.mdast, {force: true})
removePosition(file.data.hast, {force: true})
removePosition(file.data.esast, {force: true})
Copy link
Contributor Author

@slorber slorber May 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

BTW not sure this works for esast, is there a tool to remove esast positions to make exploring the AST more convenient?

I mean these nodes:

{
  start: 68,
  end: 75,
  loc: {
    start: {
      line: 4,
      column: 13,
      offset: 68
    },
    end: {
      line: 4,
      column: 20,
      offset: 75
    }
  }
}

Copy link
Member

Choose a reason for hiding this comment

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

You can try unist-util-position-from-estree.

Copy link
Member

@wooorm wooorm May 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@remcohaszing that only gets a position, it doesn’t clear start/end/loc.

@slorber I don’t think there is one, in some tests I’ve used a walker/visit and called delete for those fields

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced with:

import {visit as visitEstree} from 'estree-util-visit'

export function removePositionEsast(tree) {
  visitEstree(tree, remove)
  return tree

  function remove(node) {
    delete node.loc
    delete node.start
    delete node.end
    delete node.range
  }
}

}
} catch (error) {
const message =
error instanceof VFileMessage ? error : new VFileMessage(error)
Expand Down Expand Up @@ -104,8 +114,11 @@ export function Editor({children}) {
const defaultValue = children
const extensions = useMemo(() => [basicSetup, oneDark, langMarkdown()], [])
const [state, setConfig] = useMdx({
formatMd: false,
position: false,
gfm: false,
frontmatter: false,
directive: false,
math: false,
value: defaultValue
})
Expand Down Expand Up @@ -198,6 +211,46 @@ export function Editor({children}) {
<code>remark-math</code>
</a>
</label>
<label>
<input
checked={state.directive}
type="checkbox"
onChange={() =>
setConfig({...state, directive: !state.directive})
}
/>{' '}
Use{' '}
wooorm marked this conversation as resolved.
Show resolved Hide resolved
<a href="https://github.com/remarkjs/remark-directive">
<code>remark-directive</code>
</a>
</label>
<hr />
slorber marked this conversation as resolved.
Show resolved Hide resolved
<label>
<input
checked={state.formatMd}
type="checkbox"
onChange={() =>
setConfig({...state, formatMd: !state.formatMd})
}
/>{' '}
Use{' '}
<a href="https://mdxjs.com/packages/mdx/#optionsformat">
<code>format: &apos;md&apos;</code>
</a>
</label>
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should add a tooltip here to explain MDX infers this based on the input file extension by default. I think this option makes sense for the playground, but it’s best to discourage using this option.

Actually, maybe a radio button is more appropriate here than a checkbox, although the checkbox currently fits nice in the UI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes a radio or select can also work, open to change a bit the UI, not sure how to organize now that it's not just a list of remark plugins.

but it’s best to discourage using this option.

Why? Docusaurus uses MDX and I think supporting a commonmark mode can help existing docs sites to adopt it more easily, ie to have something less strict running asap without a ton of compilation errors to fix

Copy link
Member

Choose a reason for hiding this comment

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

yes a radio or select can also work, open to change a bit the UI, not sure how to organize now that it's not just a list of remark plugins.

Let’s wait for @wooorm’s feedback, as I think he designed the website. I think it looks great already. :)

but it’s best to discourage using this option.

Why? Docusaurus uses MDX and I think supporting a commonmark mode can help existing docs sites to adopt it more easily, ie to have something less strict running asap without a ton of compilation errors to fix

This option changes whether files are treated as commonmark or MDX. Leaving it off, means this is inferred from the file extension. It’s not about strictness (although basically anything is technically valid markdown, but MDX has syntax errors). This option is useful when the file extension is unknown (such as in this playground), but I wouldn’t recommend it otherwise. I don’t know how Docusaurus uses it exactly though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On my side, I find it helpful to understand the MDX compiler behavior when handling .md files.

I'm not really saying the format option is useful or recommended, and would be fine if instead we had the ability to customize the file name for example, as long as I can easily get the AST for an input parsed with CommonMark compat.

Although it's not recommended, Docusaurus historically has many sites that use MDX with a .md extension. One of the first unexpected things people stumble upon while upgrading to MDX 2 is that some tags (images, videos... ) do not appear anymore. See feedback here: facebook/docusaurus#8469 (reply in thread)

For this reason, it's likely I'll add an option format: 'mdx', to allow our users to upgrade their MDX version without having to rename all their files at the same time.

Copy link
Member

Choose a reason for hiding this comment

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

You can pass the filename. compile() takes a VFile compatible. In fact, I recommend always passing a filename if possible. Some plugins may depend on it. For example the JSX dev runtime depends on it.

await compile({
  value: '# Hello <strong>world</strong>!',
  path: '/home/me/hello.mdx'
})

I would also be cautious treating .md files as MDX. MDX treats certain constructs differently.

I.e. given the following content:

<div>
  foo

  bar
</div>

In markdown, this is equivalent to the following HTML:

<div> foo bar </div>

In MDX, this is equivalen to the following JSX:

<div>
  <p>foo</p>
  <p>bar</p>
</div>

I understand it’s useful to add the option for migration purposes in Docusaurus.

it’s best to discourage using this option.

I think this may have been misinterpreted. I means it’s best to discourage using this option in general, but there are definitely legitimate use cases. The playground, where there are no actual file names, is a good use case for such an option.

Copy link
Member

Choose a reason for hiding this comment

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

I think a radio toggle like this is better: https://wooorm.com/markdown-tm-language/, https://github.com/wooorm/markdown-tm-language/blob/656e7dea3f5248c728a6eeda4e9780778bd6449f/example/index.jsx#L412-L437. Don’t “connect” it to an option. But allow people to choose whether the input is plain markdown or MDX

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don’t “connect” it to an option. But allow people to choose whether the input is plain markdown or MDX

I'm not sure to understand what you mean here 😅

Updated the PR to this UI

CleanShot 2023-05-10 at 18 05 59@2x

<label>
<input
checked={state.position}
type="checkbox"
onChange={() =>
setConfig({...state, position: !state.position})
}
/>{' '}
Use{' '}
<a href="https://github.com/syntax-tree/unist#position">
<code>position</code>
</a>
slorber marked this conversation as resolved.
Show resolved Hide resolved
</label>
</form>
</TabPanel>
</Tabs>
Expand Down
56 changes: 56 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
"rehype-slug": "^5.0.0",
"rehype-stringify": "^9.0.0",
"remark-cli": "^11.0.0",
"remark-directive": "^2.0.1",
slorber marked this conversation as resolved.
Show resolved Hide resolved
"remark-frontmatter": "^4.0.0",
"remark-gemoji": "^7.0.0",
"remark-gfm": "^3.0.0",
Expand All @@ -104,6 +105,7 @@
"unified": "^10.0.0",
"unist-builder": "^3.0.0",
"unist-util-visit": "^4.0.0",
"unist-util-remove-position": "^4.0.2",
"uvu": "^0.5.0",
"vfile": "^5.0.0",
"vfile-message": "^3.0.0",
Expand Down