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

Typescript and Rehype refactor #428

Conversation

ChristianMurphy
Copy link
Member

@ChristianMurphy ChristianMurphy commented May 17, 2020

Unified, through remark and rehype is providing the translation of markdown source into react elements.
TypeScript is used to allow for catching type related bugs and automatic generation of typings files.
tsdx is mananging the build process.
Storybook is leveraged to create runnable examples which can be hosted on GitHub pages.
GitHub actions are used for continuous integration and automatic deployment of the demo site.

There is a live demo of this code running at https://christianmurphy.github.io/react-remark
and a scoped version available on the registry with npm install --save @christianmurphy/react-remark

resolves #427

Comment on lines +12 to +15
// @ts-ignore
import remarkToRehype from 'remark-rehype';
// @ts-ignore
import rehypeReact from 'rehype-react';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These packages don't have typings yet.
Work is being done upsteam to add typings.

Comment on lines +29 to +34
remarkParseOptions,
remarkToRehypeOptions,
rehypeReactOptions,
remarkPlugins = [],
rehypePlugins = [],
onError = () => {},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API is different.
It may be possible to create an adapter which is more like the current API on top of this component to smooth the transition.

Comment on lines +32 to +37
`Lift($L$) can be determined by Lift Coefficient ($C_L$) like the following equation.

$$
L = \\frac{1}{2} \\rho v^2 S C_L
$$`
);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This gives an example of embedding math in markdown, a use-case which leveraging rehype enables.

import '@testing-library/jest-dom/extend-expect';
import { Remark } from '../src';

describe('Remark', () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests focus on testing the React abstraction, remark-rehype and rehype-react have their own tests verifying content is converted as expected.

Unified, through remark and rehype is providing the translation of markdown source into react elements.
TypeScript is used to allow for catching type related bugs and automatic generation of typings files.
tsdx is mananging the build process.
storybook is leveraged to create runnable examples which can be hosted on GitHub pages.
Comment on lines +13 to +14
platform: [ubuntu-latest, windows-latest, macos-latest]
node-version: [12, 14]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does node version and cross platform testing out of the box

- name: Deploy to GitHub pages
uses: JamesIves/github-pages-deploy-action@releases/v3
with:
ACCESS_TOKEN: ${{ secrets.ACCESS_TOKEN }}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This deploys storybook to GitHub packages, it does take a bit of setup, it needs an access token to push changes to a branch.

@ChristianMurphy
Copy link
Member Author

This would also resolve several typing related issues such as: #261 #251 #341 #417

@ZupoLlask
Copy link

When is this fix going to be released?

@gilisho
Copy link
Contributor

gilisho commented Aug 7, 2020

@rexxars sorry for tagging, but I would be really happy if this could be merged, so I'm just pinging.

"unist-util-visit": "^1.4.1",
"xtend": "^4.0.1"
"rehype-react": "^5.0.1",
"remark-parse": "^8.0.2",
Copy link

@hypirion hypirion Oct 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to barge in here, but since this PR is tagged as a major release, I'm wondering if it's possible to consider a bump of remark-parse to "^9.0.0" before v5? I haven't looked too deeply into this, but given that 9.0.0 uses micromark behind the covers, it may be a bit of work.

However, if it's a minor task, I'd be glad to see it bumped as it resolves #383 and other issues I've detected related to markdown parsing (Example:

\[react\-markdown\]\(https\:\/\/remarkjs\.github\.io\/react-markdown\/\)

will not remove the backslash escapes from \: and \/, which is not compliant with the CommonMark specification).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plan earlier is explained here: #470 (comment).
It seems this PR would be extremely incompatible with the current project. So the plan was instead to mostly bump remark and fix plugins (#425) first. Then do this PR later.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks, I didn't see that one. Thanks for the quick reply and the good work here!

@wooorm wooorm closed this Oct 17, 2020
@wooorm wooorm deleted the branch remarkjs:v5 October 17, 2020 15:40
@ChristianMurphy ChristianMurphy deleted the typescript-and-rehype-refactor branch October 18, 2020 20:37
@stjepangolemac
Copy link

What happened with this?

@ChristianMurphy
Copy link
Member Author

It was closed by a quirk of GiHub, it was pointed to the v5 branch, which was merged, orphaning this PR, which GH autocloses.

An updated version of this addressing the comments from #427 (comment) and #427 (comment) is in the works, in the mean time the lower level API that it would build on is already available in https://github.com/ChristianMurphy/react-remark, the next proposal would be a wrapper/preset for react-remark, which matches the existing API more closely.

@devuxer
Copy link

devuxer commented Mar 6, 2021

I came to this pull request from #397, and I'm finding myself a little confused about the status of it. I just want my headings to have IDs so I can link to them. How can I do this today with the current version of react-markdown?

@wooorm
Copy link
Member

wooorm commented Mar 6, 2021

Still working on it: #549

@devuxer
Copy link

devuxer commented Mar 6, 2021

@wooorm , Thanks for the quick reply. and I appreciate your work on this. Does #549 mean it is simply not possible to do what I'm trying to do right now with react-markdown, or is there a stopgap/workaround I can throw into my code for now until the next version is released?

Not sure if this is related to #549 as well, but I'm noticing that markdown tables are being converted to HTML tables. I haven't looked yet to see if there's a plug-in for tables, though, so perhaps that would solve it.

@wooorm
Copy link
Member

wooorm commented Mar 6, 2021

You can overwrite the heading renderer right now. But 549 indeed means that and is about fixing all this stuff.

Tables are in remark-gfm and work

@devuxer
Copy link

devuxer commented Mar 6, 2021

@wooorm , Got it, thanks! Since sending my last message, I decided to give markdown-to-jsx a try instead, and that seems to do what I need out of the box. So, I'll go with that until #549 comes along, then I can reevaluate which library works better for my use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 area/deps This affects dependencies 📚 area/docs This affects documentation 🗄 area/interface This affects the public interface ☂️ area/types This affects typings 🧑 semver/major This is a change 💬 type/discussion This is a request for comments
Development

Successfully merging this pull request may close these issues.

None yet

9 participants