-
Notifications
You must be signed in to change notification settings - Fork 782
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
Escaped pipes in docs-readme markdown table are causing issues #1777
Comments
Looks like need reopen, change from #1779 has been lost in master |
This don't need fix in stencil, good markdown parser should work fine! |
Yeah I'm seeing this as well. Do we like that 'or' solution? I'm happy to raise a PR and switch back to that, or I can look deeper into the issue-trail on simple-markdown/markdown-to-jsx and try and understand/restore the escaping pipe. |
@adamdbradley It seems this issue needs to be reopened? Is this correct? The issue is still happening for me |
This issue was created from having issues with broken tables when documenting union type props, such as this: As @manucorporat mentioned, it depends on the parser if this is handled correctly or not. I am using Storybook to view the Stencil.js docs-readme, and I chased down the problem to be with the underlying dependcy After this fix, the tables looked liked this (so not broken anymore, but also not "pretty"): The PR I submitted, which for some reason seems to be lost, changed it to have "or" instead of "/|", which looked prettier. Sorry for the confusion this might have caused, as looking back I see that the linked PR is not covering this issue, but handling a related side-issue. Looking at the git history, it seems that this was deliberately removed here: d196971 Seeing as If yes, I propose we create a separate issue, and I can do a PR for the change. |
@dutscher what are you parsing/rendering the markdown with? |
@robaxelsen my styleguide is build with fractal and this use https://www.npmjs.com/package/marked ^0.7.0. cheers |
Stencil version:
I'm submitting a:
[x] bug report
[ ] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://stencil-worldwide.herokuapp.com/ or https://forum.ionicframework.com/
Current behavior:
When generating readme.md with the internal docs compiler, union types are output with single escaped special character
\|
:stencil/src/compiler/docs/readme/docs-util.ts
Line 35 in 30aa0c7
This is causing issue for markdown parsers, storybook.js included as seen in this issue: storybookjs/storybook#6869
Expected behavior:
Currently both
markdown-to-jsx
andsimple-markdown
libraries do not parse these escaped pipe characters correctly (nor does markdown preview in webstorm), resulting in broken tables. I except the fact that string literals when used in JavaScript regex ignores single escaped special characters, as opposed to double escaped\\|
, to be the main reason why this breaks markdown table parsing.To make it easier to parse, and to avoid workarounds or new exception case supported in popular markdown parsers such as
simple-markdown
, in a non-standard way (as JS regex can't parse single escaped special characters), I would suggest to parse union types toor
instead of\|
.Steps to reproduce:
Some simple code to illustrate, with sample markdown table generated from stencil.js, can be found here in test repo: https://github.com/robaxelsen/markdown-to-jsx-table-pipe-test
Related code:
See repo above.
Other information:
I am working on a PR for this.
The text was updated successfully, but these errors were encountered: