Add support for MDX 2 comments #11563

wooorm · 2021-09-21T18:05:50Z

Description

This adds support for MDX 2 comments (which are the same as other
comments inside JSX) in addition to MDX 1 comments (HTML-like
constructs).

Please note that this is my first PR to prettier and I don’t know the codebase well.

Related to GH-10706.
Related to wooorm/xdm#61.

/cc @thorn0 @fisker

Checklist

I’ve added tests to confirm my change works.
(If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
(If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
I’ve read the contributing guidelines.

✨Try the playground for this PR✨

This adds support for MDX 2 comments (which are the same as other comments inside JSX) in addition to MDX 1 comments (HTML-like constructs). Related to GH-10706. Related to wooorm/xdm#61.

wooorm · 2021-09-21T18:06:21Z

src/language-markdown/embed.js

+          { stripTrailingHardline: true }
+        ) +
+        "}"
+      );


I’m not sure why the MDX stuff is in embed, but that’s why I added it here

I don't think this is the right place to add it, it should be here

prettier/src/language-markdown/printer-markdown.js

Line 75 in 5aabd70

switch (node.type) {

When we disable embed language, it will throw.

Playground link

Pushed a fix to also add it to the normal printer. It looks like embed is for formatting embedded languages (so JS in MD), but not always available, hence it’s in two places?

Given they print the same result, I think we can remove this one.

Please add a test in https://github.com/prettier/prettier/tree/main/tests/format/mdx/embedded-language-formatting

Does the embedded parser perhaps break lines in comments or in other ways format them? 🤔

Okay, I was going to ask you about that... I understand that's too much for you, but I failed to update remark-parse(#9397). It will be great, if you can help us for the upgrade sometime later.

My goal is to get MDX to an RC in 2 weeks (essentially porting xdm back into it and adding docs). So we do have some time (before but also probably after it).

I can help, yeah. All of unified is now ESM-only. I believe prettier is bundling dependencies so that shouldn’t be a problem for end users. Can ESM be setup internally in prettier/src/language-markdown?

Unfortunately, no. We still don't know the problem, but we can't make ES Module work for the tests, even a simple esm file... #11235

I guess we'll have to wait until Jest have better support for modules.

Also, our migration to Pure ESM can be tracked at #10157

can the markdown language be compiled first before tests?

wooorm · 2021-09-21T18:06:58Z

src/language-markdown/pragma.js

@@ -9,6 +9,7 @@ function startWithPragma(text) {
  const regex = new RegExp(
    [
      `<!--\\s*${pragma}\\s*-->`,
+      `{\\s*\\/\\*\\s*${pragma}\\s*\\*\\/\\s*}`,
      `<!--.*\r?\n[\\s\\S]*(^|\n)[^\\S\n]*${pragma}[^\\S\n]*($|\n)[\\s\\S]*\n.*-->`,


This other regex looks complex, which is why I didn’t port that to an es-comment either

tests/format/mdx/mdx/__snapshots__/jsfmt.spec.js.snap

src/language-markdown/printer-markdown.js

fisker · 2021-09-22T07:01:55Z

@JounQin Can you please help review this?

JounQin · 2021-09-22T13:13:33Z

Will prettier support mdx@1 and mdx@2 at the same time in the future? Or is this just a temporary workaround?

wooorm · 2021-09-22T13:22:12Z

I see this as a workaround to at least not format MDX 2 comments.

I recommend to, as soon as possible, support MDX 2 only.

JounQin · 2021-09-22T13:37:24Z

tests/format/mdx/embedded-language-formatting/pull-11563.mdx

+
+{/* Some more. */}
+
+{/* Some more. */}


Is this duplicate with the above comment? Maybe a test for multiple * here?

Remove the duplicate.
Stars are not yet supported, as old remark sees them as lists. Would be nice to have that, but it’s a lot of work, which would also be solved by updating to use latest remark and remark-mdx!

A changelog should be added at https://github.com/prettier/prettier/tree/main/changelog_unreleased/mdx and mention this limitation.

How should I do this? I have no experience with this

Note that there are lots of limitations currently. xdm/mdx-js@2 allow any JS expression in there, so there are many limitations anyway, but comments (like this) solve a common use case, even though it's not perfect

sosukesuzuki

I'm not familiar with MDX, but looks good to me

fisker · 2021-09-26T04:46:54Z

@wooorm #11563 (comment)

wooorm · 2021-09-26T13:01:26Z

@fisker done. How does this look?

@JounQin What would you suggest for this changelog entry?

changelog_unreleased/mdx/11563.md

sosukesuzuki · 2021-10-05T05:33:18Z

@fisker #11563 (comment)

sosukesuzuki · 2021-10-08T06:23:43Z

We'll merge this when we start to prepare next minor release.

wooorm · 2021-10-08T07:17:28Z

Awesome!
Is there a rough time known for when that would be?

sosukesuzuki · 2021-10-08T08:15:57Z

I think it will be in early November.

wooorm · 2021-11-17T08:24:46Z

Thanks for merging this @sosukesuzuki, and everyone for the help!

kachkaev · 2022-02-01T17:55:12Z

General MDX 2 issues: #12209

gutenye · 2022-07-20T13:27:36Z

Does not support block comment

{/*

hello

*/}

* Add support for MDX 2 comments This adds support for MDX 2 comments (which are the same as other comments inside JSX) in addition to MDX 1 comments (HTML-like constructs). Related to prettierGH-10706. Related to wooorm/xdm#61. * Fix blank line before comment * Add `esComment` to `printer-markdown` * Add embedded test * Remove embedded formatter * Add support for es-comments as blocks & test prettier-ignore comments * Add a test for mdx pragma * Remove duplicate fixture * Add changelog * Add note clarifying support Co-authored-by: sosukesuzuki <aosukeke@gmail.com>

Add support for MDX 2 comments

8a38553

This adds support for MDX 2 comments (which are the same as other comments inside JSX) in addition to MDX 1 comments (HTML-like constructs). Related to GH-10706. Related to wooorm/xdm#61.

wooorm commented Sep 21, 2021

View reviewed changes

wooorm mentioned this pull request Sep 21, 2021

Cannot use <!-- for comments with xdm, but Prettier cannot use {/**/} for comments wooorm/xdm#61

Closed

4 tasks

wooorm added 2 commits September 22, 2021 08:14

Fix blank line before comment

90c3f99

Add esComment to printer-markdown

b06a928

fisker reviewed Sep 22, 2021

View reviewed changes

src/language-markdown/printer-markdown.js Outdated Show resolved Hide resolved

wooorm added 3 commits September 22, 2021 08:50

Add embedded test

eb74f5a

Remove embedded formatter

0f2e047

Add support for es-comments as blocks & test prettier-ignore comments

3c129bd

Add a test for mdx pragma

e60748f

sosukesuzuki mentioned this pull request Sep 22, 2021

Drop MDX 1 comments #11567

Open

JounQin reviewed Sep 22, 2021

View reviewed changes

Remove duplicate fixture

998b94a

sosukesuzuki approved these changes Sep 23, 2021

View reviewed changes

Add changelog

8486d3b

JounQin reviewed Sep 26, 2021

View reviewed changes

changelog_unreleased/mdx/11563.md Outdated Show resolved Hide resolved

Add note clarifying support

6874565

JounQin approved these changes Sep 26, 2021

View reviewed changes

wooorm requested a review from fisker September 29, 2021 15:35

wooorm mentioned this pull request Sep 30, 2021

Add invoice for @wooorm unifiedjs/collective#48

Merged

5 tasks

ChristianMurphy approved these changes Sep 30, 2021

View reviewed changes

sosukesuzuki added this to the 2.5 milestone Oct 6, 2021

fisker approved these changes Oct 7, 2021

View reviewed changes

Merge branch 'main' into mdx-es-comments

b383cae

sosukesuzuki merged commit cca8469 into prettier:main Nov 17, 2021

wooorm deleted the mdx-es-comments branch November 17, 2021 08:24

wooorm mentioned this pull request Nov 30, 2021

Add invoice for @wooorm unifiedjs/collective#50

Merged

5 tasks

nickrttn mentioned this pull request Feb 1, 2022

Improve MDX range ignore support #12208

Merged

4 tasks

txp1035 mentioned this pull request Aug 19, 2022

[discuss] 讨论下文档格式化的一些问题 umijs/umi#9023

Open

This comment was marked as spam.

Sign in to view

JounQin mentioned this pull request Dec 11, 2023

feat: use eslint-plugin-mdx for md/mdx files antfu/eslint-config#353

Draft

This was referenced Feb 26, 2024

kie-issues#946: DMN Editor playwright test suite - part 1 - initialization apache/incubator-kie-tools#2158

Merged

Remove prettier-ignore from all boxed-expression-component .mdx files apache/incubator-kie-issues#976

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for MDX 2 comments #11563

Add support for MDX 2 comments #11563

wooorm commented Sep 21, 2021

wooorm Sep 21, 2021

fisker Sep 22, 2021 •

edited

wooorm Sep 22, 2021

fisker Sep 22, 2021

wooorm Sep 22, 2021

fisker Sep 22, 2021

wooorm Sep 22, 2021

fisker Sep 22, 2021 •

edited

sosukesuzuki Sep 22, 2021

wooorm Sep 22, 2021

wooorm Sep 21, 2021

fisker commented Sep 22, 2021

JounQin commented Sep 22, 2021

wooorm commented Sep 22, 2021

JounQin Sep 22, 2021

wooorm Sep 22, 2021

JounQin Sep 22, 2021

wooorm Sep 24, 2021

sosukesuzuki left a comment

fisker commented Sep 26, 2021

wooorm commented Sep 26, 2021

sosukesuzuki commented Oct 5, 2021

sosukesuzuki commented Oct 8, 2021

wooorm commented Oct 8, 2021

sosukesuzuki commented Oct 8, 2021

wooorm commented Nov 17, 2021

kachkaev commented Feb 1, 2022

gutenye commented Jul 20, 2022

This comment was marked as spam.

Add support for MDX 2 comments #11563

Add support for MDX 2 comments #11563

Conversation

wooorm commented Sep 21, 2021

Description

Checklist

Choose a reason for hiding this comment

fisker Sep 22, 2021 • edited

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

fisker Sep 22, 2021 • edited

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

fisker commented Sep 22, 2021

JounQin commented Sep 22, 2021

wooorm commented Sep 22, 2021

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

sosukesuzuki left a comment

Choose a reason for hiding this comment

fisker commented Sep 26, 2021

wooorm commented Sep 26, 2021

sosukesuzuki commented Oct 5, 2021

sosukesuzuki commented Oct 8, 2021

wooorm commented Oct 8, 2021

sosukesuzuki commented Oct 8, 2021

wooorm commented Nov 17, 2021

kachkaev commented Feb 1, 2022

gutenye commented Jul 20, 2022

This comment was marked as spam.

fisker Sep 22, 2021 •

edited

fisker Sep 22, 2021 •

edited