refactor(v2): merge linkify function used in blog and docs#4391
Conversation
|
[V1] Deploy preview failure Built without sensitive environment variables with commit 845ff68 https://app.netlify.com/sites/docusaurus-1/deploys/6049ccf00b46340007f2554b |
| { | ||
| siteDir: options.siteDir, | ||
| sourceToPermalink: options.sourceToPermalink, | ||
| onBrokenMarkdownLink(brokenMarkdownLink) { | ||
| options.onBrokenMarkdownLink({ | ||
| // TODO: refactor version to contentPaths | ||
| version, | ||
| filePath: brokenMarkdownLink.filePath, | ||
| link: brokenMarkdownLink.link, | ||
| }); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
if we rename params docsDirPath, docsDirPathLocalized and property version we can change this block to options
| { | ||
| // TODO: refactor names and we can pass version here | ||
| contentPath: version.docsDirPath, | ||
| contentPathLocalized: version.docsDirPathLocalized, | ||
| }, |
There was a problem hiding this comment.
if we rename docsDirPath, docsDirPathLocalized we can simply pass here version
| onBrokenMarkdownLink(data) { | ||
| onBrokenMarkdownLink({ | ||
| // TODO: we should return here all contentPaths as we are checking them anyway | ||
| // TODO temporary, should consider the file being in localized folder! | ||
| folderPath: data.contentPaths.contentPath, | ||
| filePath: data.filePath, | ||
| link: data.link, | ||
| }); | ||
| }, |
There was a problem hiding this comment.
TODO temporary, should consider the file being in localized folder!
instead of passing folderPath se should update this code to use contentPaths and we can simply pass onBrokenMarkdownLink function here
|
Deploy preview for docusaurus-2 ready! Built without sensitive environment variables with commit 845ff68 |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4391--docusaurus-2.netlify.app/classic/ |
|
|
||
| export type BrokenMarkdownLink<T extends ContentPaths> = { | ||
| filePath: string; | ||
| contentPaths: T; |
There was a problem hiding this comment.
I'm unsure if this name is good enough, docs had version and blog contentPaths, maybe a model is a better name?
|
Closing in favor of #4402 I don't think it's too useful to add support for pages (even for blog, the usefulness is quite limited. Imho it's mostly useful for versioning/docs). |
3 participants
Motivation
This change extract code used for linking in blog and docs, to utils that it can be latter used in other plugins eg. pages
I decided to not refactor to much to ease code review, but i added few TODO that some of properties between docs and blog should be aligned.
if you prefer that properties should be renamed in this PR, I'm going to merge to this one #4402
Support for pages can/should be done in separate PR as its a feature not a refactor
docusaurus/packages/docusaurus-plugin-content-pages/src/markdownLoader.ts
Lines 14 to 19 in c32d8bd
Have you read the Contributing Guidelines on pull requests?
ay
Test Plan
there is no change in functionality and all unit test should pass, linking still should work in docs and in blog,
it can be tested by clicking in one of links in docs, eg.
https://deploy-preview-4391--docusaurus-2.netlify.app/classic/docs/contributing#get-involved