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

frontmatter is targeted #336

Closed
chenxsan opened this issue Aug 21, 2021 · 16 comments · Fixed by #337
Closed

frontmatter is targeted #336

chenxsan opened this issue Aug 21, 2021 · 16 comments · Fixed by #337
Labels
🙉 open/needs-info This needs some more info 🐛 type/bug This is a problem

Comments

@chenxsan
Copy link
Contributor

chenxsan commented Aug 21, 2021

Subject of the issue

Say I have a MDX file with this frontmatter:

---
title: Code Splitting
related:
  - title: <link rel="prefetch/preload" > in webpack
    url: https://medium.com/webpack/link-rel-prefetch-preload-in-webpack-51a52358f84c
---

Note that the <link > was not closed.

Now eslint would report Parsing error: Expected corresponding JSX closing tag for <link>. Which seems wrong to me. A related comment is here webpack/webpack.js.org#5312 (comment).

Your environment

  • OS:
  • Packages:
  • Env:

Steps to reproduce

  1. clone https://github.com/webpack/webpack.js.org
  2. open src/content/guides/code-splitting.mdx and replace the /> with > in the related part of frontmatter.

FYI, I've tried the latest version #334 (comment) with no luck.

Expected behaviour

No error.

Actual behaviour

An error was shown because frontmatter is targetted.

@chenxsan chenxsan added 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Aug 21, 2021
@JounQin
Copy link
Member

JounQin commented Aug 21, 2021

mdx is jsx like.

@chenxsan
Copy link
Contributor Author

chenxsan commented Aug 21, 2021

I'm not familiar with all those remark/eslint stuffs.

But as far as I know, the frontmatter would be transformed into something like:

export const frontmatter = {
  title: '<link >'
}

Does it really make sense to lint that part? JSX wouldn't consider that <link > in the title as an error.

@JounQin
Copy link
Member

JounQin commented Aug 21, 2021

That would depend on remark-frontmatter's behavior.

@wooorm Will the following work as same as above?

---
title: Code Splitting
related:
  - title: <link rel="prefetch/preload" /> in webpack
    url: <https://medium.com/webpack/link-rel-prefetch-preload-in-webpack-51a52358f84c>
---

It's parsed as jsx in paragraph even used with remark-frontmatter, that's the root cause.

How can I know whether jsx is in frontmatter? @wooorm

ast
{
  "type": "root",
  "children": [
    {
      "type": "thematicBreak",
      "position": {
        "start": {
          "line": 1,
          "column": 1,
          "offset": 0
        },
        "end": {
          "line": 1,
          "column": 4,
          "offset": 3
        },
        "indent": []
      }
    },
    {
      "type": "paragraph",
      "children": [
        {
          "type": "text",
          "value": "title: Code Splitting\nrelated:",
          "position": {
            "start": {
              "line": 2,
              "column": 1,
              "offset": 4
            },
            "end": {
              "line": 3,
              "column": 9,
              "offset": 34
            },
            "indent": [
              1
            ]
          }
        }
      ],
      "position": {
        "start": {
          "line": 2,
          "column": 1,
          "offset": 4
        },
        "end": {
          "line": 3,
          "column": 9,
          "offset": 34
        },
        "indent": [
          1
        ]
      }
    },
    {
      "type": "list",
      "ordered": false,
      "start": null,
      "spread": false,
      "children": [
        {
          "type": "listItem",
          "spread": false,
          "checked": null,
          "children": [
            {
              "type": "paragraph",
              "children": [
                {
                  "type": "text",
                  "value": "title: ",
                  "position": {
                    "start": {
                      "line": 4,
                      "column": 5,
                      "offset": 39
                    },
                    "end": {
                      "line": 4,
                      "column": 12,
                      "offset": 46
                    },
                    "indent": []
                  }
                },
                {
                  "type": "jsx",
                  "value": "",
                  "position": {
                    "start": {
                      "line": 4,
                      "column": 12,
                      "offset": 46
                    },
                    "end": {
                      "line": 4,
                      "column": 42,
                      "offset": 76
                    },
                    "indent": []
                  }
                },
                {
                  "type": "text",
                  "value": " in webpack\nurl: ",
                  "position": {
                    "start": {
                      "line": 4,
                      "column": 42,
                      "offset": 76
                    },
                    "end": {
                      "line": 5,
                      "column": 10,
                      "offset": 97
                    },
                    "indent": [
                      5
                    ]
                  }
                },
                {
                  "type": "link",
                  "title": null,
                  "url": "https://medium.com/webpack/link-rel-prefetch-preload-in-webpack-51a52358f84c",
                  "children": [
                    {
                      "type": "text",
                      "value": "https://medium.com/webpack/link-rel-prefetch-preload-in-webpack-51a52358f84c",
                      "position": {
                        "start": {
                          "line": 5,
                          "column": 10,
                          "offset": 97
                        },
                        "end": {
                          "line": 5,
                          "column": 86,
                          "offset": 173
                        },
                        "indent": []
                      }
                    }
                  ],
                  "position": {
                    "start": {
                      "line": 5,
                      "column": 10,
                      "offset": 97
                    },
                    "end": {
                      "line": 5,
                      "column": 86,
                      "offset": 173
                    },
                    "indent": []
                  }
                }
              ],
              "position": {
                "start": {
                  "line": 4,
                  "column": 5,
                  "offset": 39
                },
                "end": {
                  "line": 5,
                  "column": 86,
                  "offset": 173
                },
                "indent": [
                  5
                ]
              }
            }
          ],
          "position": {
            "start": {
              "line": 4,
              "column": 1,
              "offset": 35
            },
            "end": {
              "line": 5,
              "column": 86,
              "offset": 173
            },
            "indent": [
              1
            ]
          }
        }
      ],
      "position": {
        "start": {
          "line": 4,
          "column": 1,
          "offset": 35
        },
        "end": {
          "line": 5,
          "column": 86,
          "offset": 173
        },
        "indent": [
          1
        ]
      }
    },
    {
      "type": "thematicBreak",
      "position": {
        "start": {
          "line": 6,
          "column": 1,
          "offset": 174
        },
        "end": {
          "line": 6,
          "column": 4,
          "offset": 177
        },
        "indent": []
      }
    }
  ],
  "position": {
    "start": {
      "line": 1,
      "column": 1,
      "offset": 0
    },
    "end": {
      "line": 7,
      "column": 1,
      "offset": 178
    }
  }
}
    

@atesgoral
Copy link

Shouldn't the frontmatter be preprocessed and put aside as metadata before the JSX parser is unleashed on the rest of the document?

@JounQin
Copy link
Member

JounQin commented Aug 21, 2021

Shouldn't the frontmatter be preprocessed and put aside as metadata before the JSX parser is unleashed on the rest of the document?

I'm not familiar with frontmatter, let's wait @wooorm to clarify first. Personally, I don't want to add specific logic for a specific plugin.

If you have any good thought, welcome to PR to preview first.

@wooorm
Copy link
Member

wooorm commented Aug 21, 2021

Frontmatter, often in YAML, is frequently used in combination with markdown/MDX: https://github.com/wooorm/xdm#frontmatter. While ESM import/exports are powerful, frontmatter is static: it can be access without compiling and evaluating the file.
GitHub, prettier, and many blog platforms support it. mdx-bundler does, too.


But as far as I know, the frontmatter would be transformed into something like:

export const frontmatter = {
  title: '<link >'
}

@chenxsan

Not necessarily, frontmatter has always worked with a plugin, but exposing it as an export only started working since xdm, and only when combined with remcohaszing/remark-mdx-frontmatter. But it’s really smart and powerful so I can understand it becoming quite popular.

Will the following work as same as above?
@JounQin

If remark-frontmatter is turned on (with YAML, which is the default), then yes, that parses YAML frontmatter, not as JSX.

It's parsed as jsx in paragraph even used with remark-frontmatter, that's the root cause.
@JounQin

It sounds like remark-frontmatter isn’t used correctly in your test case? It should work: https://astexplorer.net/#/gist/85406af039218fbe83c53f831da662be/244e40f3f5208d9c11922e8b3885c5095afbe0a7 (this example is just remark, but I’m pretty sure MDX parsing doesn’t conflict)

Shouldn't the frontmatter be preprocessed and put aside as metadata before the JSX parser is unleashed on the rest of the document?
@atesgoral

Yes, I agree. frontmatter in YAML is really strict. It is highly unlikely that someone does not want frontmatter but still starts a document with those three dashes.
Frontmatter is pretty simple to parse (compared to say, JSX), so there shouldn’t be much added code weight either.
Alternatively, it could make it an option.

@JounQin
Copy link
Member

JounQin commented Aug 21, 2021

@wooorm

It sounds like remark-frontmatter isn’t used correctly in your test case?

It is using remark@13 at AST explorer, which is not supported by eslint-mdx currently.

My test case is remark-frontmatter@2, is this related?

@wooorm
Copy link
Member

wooorm commented Aug 21, 2021

I believe remark-frontmatter@2 works with remark@12. remark-frontmatter@3 and remark-frontmatter@4 work with remark@13 and remark@14? 🤔 It does sound related indeed.

@JounQin
Copy link
Member

JounQin commented Aug 21, 2021

@wooorm I mean will the parsed AST be same between remark-frontmatter@2 vs remark-frontmatter@3/4?

@wooorm
Copy link
Member

wooorm commented Aug 21, 2021

Yes, the AST is the same with remark-frontmatter 1 through 4, and most likely all future versions as well. The MDX AST did you change. But I do not see that change interfering with frontmatter.

@JounQin
Copy link
Member

JounQin commented Aug 21, 2021

@wooorm Does the plugins order matter? In eslint-mdx , it uses remark-mdx first then other user customed plugins.


OK, I find the problem, the custom remark config is only used for mdx/remark rule, this can be improved.

@JounQin
Copy link
Member

JounQin commented Aug 21, 2021

@chenxsan Please try yarn add -D https://pkg.csb.dev/mdx-js/eslint-mdx/commit/478660c5/eslint-plugin-mdx

@chenxsan
Copy link
Contributor Author

image

I'm still getting the error.

@JounQin
Copy link
Member

JounQin commented Aug 21, 2021

@chenxsan Did you add remark-frontmatter@2 in your remark config?

@chenxsan
Copy link
Contributor Author

@chenxsan Did you add remark-frontmatter@2 in your remark config?

Oops, I just noticed that I have to setremarkConfig in package.json. Yes, it works with remark-frontmatter@2, but not with v3.

@JounQin
Copy link
Member

JounQin commented Aug 21, 2021

@chenxsan

See #336 (comment), remark-frontmatter@2 is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙉 open/needs-info This needs some more info 🐛 type/bug This is a problem
Development

Successfully merging a pull request may close this issue.

4 participants