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

fix(v2): optimize markdown parser regex (fix #4726) #4729

Merged
merged 2 commits into from
May 5, 2021

Conversation

nam-hle
Copy link
Contributor

@nam-hle nam-hle commented May 5, 2021

Motivation

Fix #4726 (origin: #4713)

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

  1. Clone my prepared repo and install
  2. Start. Everything should work well
  3. Incremental uncomment import statements in docs/intro.md, the compilation time will increase exponentially from around 15 imported components.
  4. Apply the changes in packages/docusaurus-utils/src/markdownParser.ts of this PR into the corresponding compiled file (at client-docusaurus/node_modules/@docusaurus/utils/lib/markdownParser.js) of my repo.
  5. Do the step 2-3 again to see the compile time improvement.

Please feel free to suggest any concrete performat tests that need to do.

Related PRs

N/A.

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label May 5, 2021
@netlify
Copy link

netlify bot commented May 5, 2021

@github-actions
Copy link

github-actions bot commented May 5, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 66
🟢 Accessibility 96
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-4729--docusaurus-2.netlify.app/

@nam-hle nam-hle marked this pull request as draft May 5, 2021 03:46
@nam-hle nam-hle marked this pull request as ready for review May 5, 2021 08:00
@slorber slorber added the pr: bug fix This PR fixes a bug in a past release. label May 5, 2021
@slorber
Copy link
Collaborator

slorber commented May 5, 2021

Thanks! I tried to hack around a bit locally and also found a similar potential solution.

That seems good enough to merge for now!

Seems to work, and could see your test being very slow without the regex change.

Not sure why but it seems to only affect the 2nd regex (alternateTitleMatch), not the first one.

Hey, if you are a regex pro and want to help, any improvement to support such syntax would be welcome, as the current regex does not cover edge cases:

          import Component1 from '@site/src/components/Component1';
          import Component2, {NamedImport, SecondImport} from '@site/src/components/Component2';
          import {Component3, NamedImport as NamedImportRenamed} from '@site/src/components/Component3';
          import * as Component4 from '@site/src/components/Component4';

Thanks ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

parseMarkdownContentTitle: extremely slow Regex
4 participants