fix(mdxast): render props parse case

[pedronauck]

This pr just add mergeNodeWithoutCloseTag method in the mdxast parser that check if some node has just the element open tag without close it, then merging values into the correct node making a reverse iteration over the tree and deleting incorreting nodes.

I stressed some cases with a new snapshot test and looking over it is everything going fine!

This closes #139, closes #136 and closes #137

[timneutkens]

Could you use babel to parse the result? It's much more reliable than having a snapshot test that will be guaranteed to fail on every little change.

[pedronauck]

good, I can do that ✌️

[timneutkens]

👌

[pedronauck]

@timneutkens do you think that just this case is fine?

[timneutkens]

I wonder if there's a reason as to why remark doesn't parse multi-paragraph html / if there's an easier way to fix this. cc @wooorm.

Example:

<Foo>
  {() => {
    const bar = 'bar'
    return (
      <Bar bar={bar} />
    )
  }}
</Foo>

compiles to:

{
  "type": "root",
  "children": [
    {
      "type": "html",
      "value": "<Foo>\n  {() => {\n    const bar = 'bar'\n    return (\n      <Bar bar={bar} />\n    )\n  }}\n</Foo>",
      "position": {
        "start": {
          "line": 1,
          "column": 1,
          "offset": 0
        },
        "end": {
          "line": 8,
          "column": 7,
          "offset": 93
        },
        "indent": [
          1,
          1,
          1,
          1,
          1,
          1,
          1
        ]
      }
    }
  ],
  "position": {
    "start": {
      "line": 1,
      "column": 1,
      "offset": 0
    },
    "end": {
      "line": 8,
      "column": 7,
      "offset": 93
    }
  }
}

Whereas

<Foo>
  {() => {
    const bar = 'bar'

    return (
      <Bar bar={bar} />
    )
  }}
</Foo>

Compiles to

{
  "type": "root",
  "children": [
    {
      "type": "html",
      "value": "<Foo>\n  {() => {\n    const bar = 'bar'",
      "position": {
        "start": {
          "line": 1,
          "column": 1,
          "offset": 0
        },
        "end": {
          "line": 3,
          "column": 22,
          "offset": 38
        },
        "indent": [
          1,
          1
        ]
      }
    },
    {
      "type": "code",
      "lang": null,
      "value": "return (\n  <Bar bar={bar} />\n)",
      "position": {
        "start": {
          "line": 5,
          "column": 1,
          "offset": 40
        },
        "end": {
          "line": 7,
          "column": 6,
          "offset": 82
        },
        "indent": [
          1,
          1
        ]
      }
    },
    {
      "type": "paragraph",
      "children": [
        {
          "type": "text",
          "value": "  }}\n",
          "position": {
            "start": {
              "line": 8,
              "column": 1,
              "offset": 83
            },
            "end": {
              "line": 9,
              "column": 1,
              "offset": 88
            },
            "indent": [
              1
            ]
          }
        },
        {
          "type": "html",
          "value": "</Foo>",
          "position": {
            "start": {
              "line": 9,
              "column": 1,
              "offset": 88
            },
            "end": {
              "line": 9,
              "column": 7,
              "offset": 94
            },
            "indent": []
          }
        }
      ],
      "position": {
        "start": {
          "line": 8,
          "column": 1,
          "offset": 83
        },
        "end": {
          "line": 9,
          "column": 7,
          "offset": 94
        },
        "indent": [
          1
        ]
      }
    }
  ],
  "position": {
    "start": {
      "line": 1,
      "column": 1,
      "offset": 0
    },
    "end": {
      "line": 9,
      "column": 7,
      "offset": 94
    }
  }
}

Results taken from https://astexplorer.net/

[wooorm]

@timneutkens It does “block” HTML if it’s a known HTML block element. Would it be possible to detect the imported components, and pass their names to the processor as “blocks”?

[timneutkens]

@wooorm we kind of hijacked that option https://github.com/mdx-js/mdx/blob/master/packages/mdx/index.js#L11 to use a regex 🕵️

So I guess astexplorer is not a great example 😅

[pedronauck]

I think that remark doesn't parse multi-paragraph because it assumes that document is just a html, that no reason to use a function inside it like in jsx! But render a component with some render callbacks is something that the community are embracing a lot, limit or create some workarounds in think that isn't a good option. IMHO, the better choice would be the language works like expect!

[timneutkens]

@pedronauck the thing is that anything inside jsx tags is not some special case, it should just be 1 big jsx block meaning it'd work fine if the parser turns it into that. But I'll have to check the remark code again, specifically the part @wooorm was pointing out, since in that case we don't even have to change that much on our side of the parser 😄

doesn't parse multi-paragraph because it assumes that document is just a html, that no reason to use a function inside it like in jsx!

Note that remark just says: everything inside html is text, so it has nothing to do with it having to know what the content is, since mdx doesn't parse jsx tags, it just puts it into the final document without touching it.

[pedronauck]

I agree in some points, but I think that mdxast is assuming the jsx parser works here, that is what remark/rehype does for html. Assuming this parser position has a lot of trade-offs that you need to consider because jsx is totally different and has a lot of implications that a simple html parser doesn't need to care about. If you Just replace an ast node that is { type: html } to { type: jsx } you're assuming that both works in the same way, and we now that isn't the case...

[pedronauck]

Nothing more about that @timneutkens ?

[renatorib]

What is missing to merge this PR?

[pedronauck]

Close in favor of the new algorithms that extends remark parser