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

Refactor + add more tests (Part 2) #854

Merged
merged 11 commits into from
Jul 16, 2018
Merged

Conversation

endiliey
Copy link
Contributor

@endiliey endiliey commented Jul 15, 2018

Changes

Add babel-register on versions.js and write-translations.js to enable transpiling ES6

Fix #851

Refactor duplicate finding docs file logic in server.js and generate.js as function getFile + add test for it

https://github.com/facebook/Docusaurus/blob/03e237abda0600768d29be9d1cb50c6fb7ddd31b/lib/server/generate.js#L86-L102
https://github.com/facebook/Docusaurus/blob/03e237abda0600768d29be9d1cb50c6fb7ddd31b/lib/server/server.js#L172-L189

Refactor the logic to find metadata to be more declarative & efficient

Before
https://github.com/facebook/Docusaurus/blob/03e237abda0600768d29be9d1cb50c6fb7ddd31b/lib/server/server.js#L156-L161
https://github.com/facebook/Docusaurus/blob/03e237abda0600768d29be9d1cb50c6fb7ddd31b/lib/server/server.js#L165

After

const metadata = Metadata[Object.keys(Metadata).find(id => Metadata[id].permalink === url)];

Refactor duplicate getting docs component logic in server.js and generate.js to a new file docs.js as function getComponent

https://github.com/facebook/Docusaurus/blob/03e237abda0600768d29be9d1cb50c6fb7ddd31b/lib/server/server.js#L195-L247
https://github.com/facebook/Docusaurus/blob/03e237abda0600768d29be9d1cb50c6fb7ddd31b/lib/server/generate.js#L110-L143

Changed the way we replace any links to markdown files to their website html links + ADD TEST

Note that mdToHtml is the mapping of a markdown to it's respective html link.
mdToHtml keys ~ number of docs

const mdToHtml = {
  'doc1.md': 'docs/en/next/doc1.html',
  'doc2.md': 'docs/en/next/doc2.html',
  'doc3.md': 'docs/en/next/doc3.html',
  'doc4.md': 'docs/en/next/doc4.html',
}

Before
It will try every possible mdToHtml key and try to replace that key with the corresponding website html links. Lot of time wasted building htmlLink from mdLink that might not exist on that file as well

Time Complexity ~ O(number of mdHtml keys)

https://github.com/facebook/Docusaurus/blob/03e237abda0600768d29be9d1cb50c6fb7ddd31b/lib/server/generate.js#L115-L131

After
It will try to find all the markdown links in a file and try to replace it with the corresponding website html links.

Time Complexity ~ O(number of unique markdown links in a file)

function mdToHtmlify(oldContent, mdToHtml, metadata) {
  let content = oldContent;
  const mdLinks = [];
  
  // find any links to markdown files
  const regex = /(?:\]\()(?:\.\/)?([^'")\]\s>]+\.md)/g;
  let match = regex.exec(content);
  while (match !== null) {
    mdLinks.push(match[1]);
    match = regex.exec(content);
  }

  // replace to their website html links 
  new Set(mdLinks).forEach(mdLink => {
    let htmlLink = mdToHtml[mdLink];
    if (htmlLink) {
      htmlLink = getPath(htmlLink, siteConfig.cleanUrl);
      htmlLink = htmlLink.replace('/en/', `/${metadata.language}/`);
      htmlLink = htmlLink.replace(
        '/VERSION/',
        metadata.version && metadata.version !== env.versioning.defaultVersion
          ? `/${metadata.version}/`
          : '/'
      );
      content = content.replace(
        new RegExp(`\\]\\((\\./)?${mdLink}`, 'g'),
        `](${htmlLink}`
      );
    }
  });
  return content;
}

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

  1. Test Locally
yarn start

Links still work as per normal

  1. Test in netlify
    https://deploy-preview-854--docusaurus-preview.netlify.com/

Related PRs

#847

@endiliey endiliey added ✅ test plan better engineering Not a bug or feature request labels Jul 15, 2018
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jul 15, 2018
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Jul 15, 2018

Deploy preview for docusaurus-preview ready!

Built with commit 64cb4f3

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

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.

@endiliey You deserve a medal 🏅

@yangshun yangshun merged commit 9f718a5 into facebook:master Jul 16, 2018
@endiliey endiliey deleted the refactor branch July 16, 2018 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
better engineering Not a bug or feature request CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants