-
Notifications
You must be signed in to change notification settings - Fork 42
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
feat: sort when syncing by parent #973
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh wow this is a very sophisticated approach, love it! i have some feedback + questions below, happy to pair on any of this!
src/lib/syncDocsPath.ts
Outdated
const doc = frontMatter(fsSync.readFileSync(filePath, 'utf8')); | ||
const slug = path.basename(filePath).replace(extensionRegexp, ''); | ||
const parentDocSlug = doc.data.parentDocSlug; | ||
|
||
return { filePath, slug, parentDocSlug }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/lib/syncDocsPath.ts
Outdated
|
||
const dependencies = Object.values(filesBySlug).reduce( | ||
(edges, obj) => { | ||
if (obj.parentDocSlug) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i know we have users that sync only parts of their docs using rdme
, i think we should probably also ensure that the parent file actually exists
(also let me know if i'm misunderstanding how this works!)
if (obj.parentDocSlug) { | |
if (obj.parentDocSlug && filesBySlug[obj.parentDocSlug]) { |
src/lib/syncDocsPath.ts
Outdated
@@ -127,6 +130,40 @@ async function pushDoc( | |||
}); | |||
} | |||
|
|||
function sortFiles(files: string[], { allowedFileExtensions }: { allowedFileExtensions: string[] }): string[] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function appears to use parentDocSlug
, is there any way we add basic support for parentDoc
? i know it uses object IDs so there's no way for us to build a graph, but at the very least we could make it so docs that have a parentDoc
attribute get synced after ones that lack one?
afe367d
to
86e7ae1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const { content, data, hash, slug } = readDoc(filePath); | ||
const { content, data, filePath, hash, slug } = fileData; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this refactor (i.e., decoupling the readDoc
so we can read the contents prior to sorting) is very clever!
Co-authored-by: Kanad Gupta <[email protected]>
Co-authored-by: Kanad Gupta <[email protected]>
0d41b9a
to
5c95884
Compare
# [9.0.0-next.11](v9.0.0-next.10...v9.0.0-next.11) (2024-02-15) ### Features * sort when syncing by parent ([#973](#973)) ([9795840](9795840)) [skip ci]
🧰 Changes
When syncing docs, it sorts them by their parents!
Users reported issues where if a parent document was in a directory, it could error out because it would get synced after one of its children.
🧬 QA & Testing
Not sure 😬.