-
Notifications
You must be signed in to change notification settings - Fork 833
chore(markdown): move out of md-to-react-email #2569
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
Conversation
🦋 Changeset detectedLatest commit: 8cf06ac The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
@cubic-dev-ai review |
@gabrielmfern I've started the AI code review. It'll take a few minutes to complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 6 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="packages/markdown/src/utils/parse-css-in-js-to-inline-css.ts">
<violation number="1" location="packages/markdown/src/utils/parse-css-in-js-to-inline-css.ts:41">
`lineHeight` must stay unitless. Appending `px` turns values like `1.5` into `1.5px`, collapsing line-height and breaking layout.</violation>
</file>
<file name="packages/markdown/src/markdown.tsx">
<violation number="1" location="packages/markdown/src/markdown.tsx:100">
Image attributes are interpolated without escaping, so characters like quotes in the alt or title break the attribute boundary and allow HTML/script injection. Please HTML-escape attribute values (src/title/alt) before embedding them.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai
to give feedback, ask questions, or re-run the review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 6 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="packages/markdown/src/utils/parse-css-in-js-to-inline-css.ts">
<violation number="1" location="packages/markdown/src/utils/parse-css-in-js-to-inline-css.ts:41">
Please treat numeric lineHeight as unitless; adding it to the px allowlist forces values like 1.5 to become 1.5px and collapses layout spacing.</violation>
</file>
<file name="packages/markdown/src/styles.ts">
<violation number="1" location="packages/markdown/src/styles.ts:57">
Spreading `codeInline` into `codeBlock` carries over `display: 'inline'`, so the `<pre>` emitted by the renderer now renders inline instead of block, breaking code block layout.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai
to give feedback, ask questions, or re-run the review.
28e03a4
to
49023ec
Compare
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
This addresses some comments on #2335.
Currently, our Markdown package heavily relies on
md-to-react-email
, this makes it much harder for us to fix issues on the Markdown component, and makes it significantly harder for the community to contribute to the component.This pull requests moves in most of the code from
md-to-react-email
that we needed, while updating the dependency onmarked
(which actually fixes the issue), and simplifying the code a bit more. This move shouldn't bring much of a maintenance burden to us, becausemd-to-react-email
was more of a thin-wrapper aroundmarked
which actually does much of the heavy lifting.In the future I think we need to find some way to converting the Markdown content into actual React elements, using our components for a better compatibility, and probably easier maintenance.
Summary by cubic
Moved Markdown rendering in-house by replacing md-to-react-email with a custom marked-based renderer. This fixes the Markdown issue and makes the component easier to maintain and contribute to.
Refactors
Bug Fixes
<Markdown />
#2335 by updating the parser and handling all elements with the custom renderer.