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.

Sign up for GitHub

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

Jump to bottom

feat(v2): support rightToc, emoji, slug for docs #1382

Merged
merged 9 commits into from
Apr 23, 2019
Merged

feat(v2): support rightToc, emoji, slug for docs #1382

merged 9 commits into from
Apr 23, 2019

Conversation

endiliey
Copy link
Contributor

@endiliey endiliey commented Apr 23, 2019 •
edited
Loading

Motivation

  • Support emoji. Example: :dog: :+1: -> 🐶 👍
  • Support slug (GitHub slug way)
  • Support right TOC like in v1
    image

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

  • Locally and production no errors
  • Tests added for rightToc works. See snapshots for structure

TODO:

  • rereview & audit code
  • fix styling

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Apr 23, 2019
@endiliey endiliey marked this pull request as ready for review April 23, 2019 07:40
@endiliey endiliey requested a review from yangshun as a code owner April 23, 2019 07:40
@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-2 ready!

Built with commit d7a28f5

https://deploy-preview-1382--docusaurus-2.netlify.com

@facebook facebook deleted a comment from docusaurus-bot Apr 23, 2019
@facebook facebook deleted a comment from docusaurus-bot Apr 23, 2019
@facebook facebook deleted a comment from docusaurus-bot Apr 23, 2019
@facebook facebook deleted a comment from docusaurus-bot Apr 23, 2019
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Apr 23, 2019 •
edited
Loading

Deploy preview for docusaurus-preview ready!

Built with commit 47add6f

https://deploy-preview-1382--docusaurus-preview.netlify.com

yangshun
yangshun approved these changes Apr 23, 2019
View reviewed changes
Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

I didn't review the parsing code too thoroughly but if it works it's fine. This is not a crucial architectural change so it's probably ok to ship potentially buggy implementations. Typically I'd avoid using babel traversal if I could, but if the perf didn't regress too much it's probably ok. Did the startup time change by much?

packages/docusaurus-mdx-loader/src/rightToc/__tests__/fixtures/name-exist.mdx Show resolved Hide resolved
packages/docusaurus-mdx-loader/src/rightToc/__tests__/index.test.js Outdated Show resolved Hide resolved
packages/docusaurus-mdx-loader/src/rightToc/__tests__/index.test.js Outdated Show resolved Hide resolved
packages/docusaurus/lib/default-theme/DocBody/index.js Outdated Show resolved Hide resolved
packages/docusaurus/lib/default-theme/DocBody/index.js Outdated Show resolved Hide resolved
@endiliey
Copy link
Contributor Author

I had to use babel because technically what this is doing is not just MDAST, but MDXAST (MDAST + JSX).
The babel traverse is just to check if there already exist some statement in markdown like export const rightToc so that I can replace it. Otherwise there will be double statement.

@endiliey
Copy link
Contributor Author

Startup time still very fast !! hehe

image

@endiliey endiliey merged commit 745f32b into facebook:master Apr 23, 2019
@endiliey endiliey deleted the mdx-loader branch April 23, 2019 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yangshun yangshun yangshun approved these changes

Assignees
No one assigned
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@endiliey @docusaurus-bot @yangshun @facebook-github-bot