Implement live preview for pages in site.json#1514
Conversation
jonahtanjz
changed the title
jonahtanjz
changed the title
ang-zeyu
left a comment
ang-zeyu
left a comment
There was a problem hiding this comment.
Thanks @jonahtanjz
Some nits:
EDIT: After looking around at the other handlers (addHandler and removeHandler), it seems all pages should be rebuilt if the added/removed file is a page/dependency of page.
This has been existing behaviour for a long while already but can probably be made into an issue 🙂
| pagesToUpdate.forEach((pageToUpdate) => { | ||
| this.pages.forEach((page, index) => { | ||
| if (page.pageConfig.src === pageToUpdate.src) { | ||
| const newPage = this.createPage({ |
There was a problem hiding this comment.
how about something like a mapAddressablePageToPage function to reduce duplication with mapAddressablePagesToPages (or if could generalise the latter maybe even better)
There was a problem hiding this comment.
Hmm I tried to combine them but it seems like their logic is quite different. As a workaround, I've extracted out the createPage part into a separate function so that both mapAddressablePagesToPages and this can call the new function which resolves the repetition issue. Is this ok?
| } else { | ||
| this.updatePages(editedPages); | ||
| const siteConfigDirectory = path.dirname(path.join(this.rootPath, this.siteConfigPath)); | ||
| this.rebuildAffectedSourceFiles(editedPages.map(page => path.join(siteConfigDirectory, page.src))); |
There was a problem hiding this comment.
hmm... would it be correct to instead use regenerateAffectedPages (in _rebuildAffectedSourceFiles) directly? (or something even lower level)
The filePaths parameter of rebuildAffectedSourceFiles indicates changed source files, but over here its the page configuration that changed, not the file. So the extra fluff (e.g. layout building) in _rebuildAffectedSourceFiles shouldn't be needed, and even with regenerateAffectedPages there's still quite a bit of fluff (e.g. change in a page's properties wouldn't affect other pages even if said page's source file is a dependency of those other pages).
Only exactly the pages which's properties have changed in site.json should be rebuilt. (and even then if using markbind s -o only the page being viewed should be rebuilt)
If this needs a substantial addition here that's not worth it, we can do it after enhancing the rebuild process for page addition / removal (above comment) + #1513 though, then streamlining the overall flow, which should make this easier; also since this is a feature addition.
There was a problem hiding this comment.
hmm... would it be correct to instead use regenerateAffectedPages (in _rebuildAffectedSourceFiles) directly? (or something even lower level)
The filePaths parameter of rebuildAffectedSourceFiles indicates changed source files, but over here its the page configuration that changed, not the file. So the extra fluff (e.g. layout building) in _rebuildAffectedSourceFiles shouldn't be needed, and even with regenerateAffectedPages there's still quite a bit of fluff (e.g. change in a page's properties wouldn't affect other pages even if said page's source file is a dependency of those other pages).
Yup agree that changing page properties wouldn't affect other pages and they should not be rebuilt. I will look into this and perhaps open another PR to work on it as it seems like it will take some time and will require quite a bit of change. For now I've switched to using regenerateAffectedPages as suggested.
3 participants
What is the purpose of this pull request?
Overview of changes:
Addresses part of #48.
This PR intends to support live preview for the
pagesproperty insite.json. Whensite.jsonis edited,reloadSiteConfigfunction will read in the new values and check for changes inpages. If there is added/removed pages, all pages will be rebuilt. This implementation is similar to setting the glob to track all.mdand.mbdfiles inpages("glob": ["**/index.md", "**/*.+(md|mbd)"]). If new pages are added/removed, all pages will be rebuilt in theaddHandler.If the attributes of a page are edited, then only that specific page/pages dependent on this will be rebuilt. For example, if the
"title"of a page atcontents/index.mdis changed, then onlycontents/index.htmlwill be regenerated. The rest of the pages will not be rebuilt.Anything you'd like to highlight / discuss:
Current implementation will rebuild all pages when there is a change in
pages.Not sure if I should only 'watch' for added/removed pages and then only update the relevant pages that will be affected.EDIT: After looking around at the other handlers (
addHandlerandremoveHandler), it seems all pages should be rebuilt if the added/removed file is a page/dependency of page.Testing instructions:
markbind servepagesproperty insite.jsonProposed commit message: (wrap lines at 72 characters)
Implement live preview for pages in site.json
When pages are added or removed from site.json, the respective
generated pages are not built or removed from the live preview.
Similarly, edited pages attributes are not reflected as well. A server
reboot is necessary to display the new changes.
Let's support live preview for pages in site.json so that the user
does not have to restart the server each time the pages property
is edited.
Checklist: ☑️