fix: resolve output folder relative to book root instead of cwd#494
Merged
fix: resolve output folder relative to book root instead of cwd#494
Conversation
When running `honkit serve` from a parent directory (e.g., `cd /parent && honkit serve ./mybook ./output`), the output folder was being resolved relative to process.cwd() instead of the book root. This caused the output folder to be outside the watch directory, making it impossible for the file watcher to correctly ignore it, resulting in infinite rebuild loops. The fix ensures the output folder is always resolved relative to the book root, so it can be properly added to the watcher's ignore list. Fixes #491
When running `honkit serve`, the following changes now trigger a full
rebuild instead of an incremental build:
1. SUMMARY.md changes - Book structure has changed, need to regenerate
all pages to reflect new navigation
2. GLOSSARY.md changes - Glossary definitions affect all pages
3. book.json/book.js changes - Configuration affects entire book
4. New file additions - New assets or pages need to be recognized
This fixes the issue where:
- Files added to SUMMARY.md during serve were not generated
- New files (assets) added during serve were not copied to output
The watch callback now receives the event type ("add", "change", etc.)
to distinguish between file modifications and new file creations.
Related to #491
- Refactor shouldFullRebuild to accept eventType parameter - Add isStructureFile helper for cleaner separation - Add integration tests for: - SUMMARY.md change triggering full rebuild - New file addition triggering full rebuild - Regular file change triggering incremental build - Build output verification with snapshots - Watch event detection The tests verify that the actual shouldFullRebuild function behaves correctly, not a duplicate of the logic in test code.
- Rename test file to better reflect its purpose (tests build output) - Remove shouldFullRebuild tests (already in shouldFullRebuild.test.ts) - Remove watch event tests (already in watch.test.ts) - Keep only black-box integration tests using bin.run The integration tests now focus on CLI input/output verification: - Files added to SUMMARY.md are generated correctly - Assets not in SUMMARY.md are copied - Output structure matches expected snapshot
Tests that serve correctly: - Generates new pages when SUMMARY.md is updated during serve - Copies new asset files when added during serve Uses subprocess with port 0 (OS-assigned) to avoid port conflicts.
There was a problem hiding this comment.
Pull request overview
This pull request fixes an infinite rebuild loop issue (#491) that occurred when running honkit serve from a parent directory with a custom output folder path. The core fix ensures that output folder paths are resolved relative to the book root instead of the current working directory, allowing the file watcher to properly ignore the output directory.
Changes:
- Fixed output folder resolution to use book root instead of process.cwd()
- Added eventType parameter to watch callback for distinguishing file change types
- Implemented shouldFullRebuild logic to determine when full vs incremental rebuilds are needed
- Added comprehensive unit and integration tests for the new functionality
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/honkit/src/cli/getOutputFolder.ts | Core fix: resolve output folder relative to book root instead of process.cwd() |
| packages/honkit/src/cli/watch.ts | Added WatchEventType export and eventType parameter to callback signature |
| packages/honkit/src/cli/shouldFullRebuild.ts | New utility to determine when full rebuild is needed based on file type and event |
| packages/honkit/src/cli/serve.ts | Integrated shouldFullRebuild logic and added livereload trigger to full rebuild path |
| packages/honkit/src/cli/tests/shouldFullRebuild.test.ts | Comprehensive unit tests for shouldFullRebuild functionality |
| packages/honkit/src/cli/tests/serve-integration.test.ts | Integration tests verifying serve rebuild behavior |
| packages/honkit/src/cli/tests/build-integration.test.ts | Integration tests verifying build output |
| packages/honkit/src/cli/tests/snapshots/build-integration.test.ts.snap | Snapshot tests for build output HTML structure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- getOutputFolder: explicitly handle absolute paths with path.isAbsolute() - getOutputFolder: add unit tests for relative, absolute, and default paths - serve: remove unused WatchEventType import - serve: extract triggerLiveReload helper to reduce code duplication - shouldFullRebuild: handle unlink events for file deletions - shouldFullRebuild: add tests for unlink event handling Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Use path.resolve() to generate absolute paths that work on both Unix and Windows platforms. Co-Authored-By: Claude Opus 4.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixed an issue where running
honkit servefrom a parent directory (e.g.,cd /parent && honkit serve ./mybook ./output) caused the output folder to be resolved relative to process.cwd() instead of the book root. Also improved rebuild logic to trigger full rebuilds for SUMMARY.md changes and new file additions.Changes
Breaking Changes
None
Test Plan
packages/honkit/src/cli/__tests__/serve-integration.test.tspackages/honkit/src/cli/__tests__/shouldFullRebuild.test.tshonkit serve ./book ./outputfrom a parent directory and confirm no infinite rebuild loops occurFixes #491