Structurally refactor Site class into Facade and separate Managers #2775

gerteck · 2026-01-14T06:49:38Z

What is the purpose of this pull request?

Overview of changes:

Closes #2774
Closes #1756

Anything you'd like to highlight/discuss:
The Site class had become a "God Class" with many responsibilities, making it hard to maintain and test.

This PR refactors and decouples the monolithic Site god class class into a Facade provided by Site/index.ts. It delegates responsibilities to four newly created manager classes (SiteAssetsManager, SitePagesManager, SiteDeployManager, and SiteGenerationManager), which are currently defined within the same file.

Organize classes in Site/index.ts for git history, tracking before evict to separate files.
Extract responsibilities into dedicated manager classes
- SiteAssetsManager: Asset handling (copying, building, removing).
  - Responsibility: Copying fonts, CSS, JS bundles, and bootstrap themes to the output directory. Handling ignore rules for assets.
- SitePagesManager: Page collection and dependency management.
  - Responsibility: Collecting addressable pages from globs, creating Page objects, parsing frontmatter for configuration, and mapping URLs.
- SiteDeployManager: GH Pages and others deployment logic.
  - Responsiblity: Pushing the artifacts in _site to a remote git branch (GitHub Pages).
- SiteGenerationManager: Core build orchestration (generate, rebuild, lazy loading).
  - Responsibility: Orchestrating the build process (generate, buildSourceFiles), managing lazy loading logic, re-building affected pages, and writing site data.

Refactor Site class as Facade, delegate logic to underlying managers
Maintain public API compatibility for now.

Separate testcases for better maintainability.

Additionally, using Site class as a Facade now enables us to hide some of the methods that are not used and not overexpose internal methods. Instead, we can easily see the key interfaces the core package (Site object) exposes for consumers to use.

Previously, the Site class exposed numerous internal methods, allowing external manipulation of internal state and obscuring the actual public API. This change reduces the surface area of the Site class, making the core package easier to maintain and consume.

Testing instructions:

npm run test

(Plan to do a merge-commit):

Proposed commit message: (wrap lines at 72 characters)
Refactor Site class into Facade and specialized Managers

Decouples monolithic Site class by extracting logic into Managers
The Site class has too many responsibilities, hard to maintain, test.
Delegate specific responsibilities to created manager classes:

SiteAssetsManager: Handles asset copying, building, and removal.
SitePagesManager: Manages page collection, mapping, and dependencies.
SiteDeployManager: Handles deployment logic (e.g., GitHub Pages).
SiteGenerationManager: Orchestrates build and regenerate processes.
The Site class now acts as an entry point (Facade), exposing only the
necessary public API methods while hiding internal state and logic.

This separation of concerns significantly reduces complexity of core
package and improves testability with dedicated test cases for each manager.

Checklist: ☑️

Updated the documentation for feature additions and enhancements
Added tests for bug fixes or features
Linked all related issues
No unrelated changes

Reviewer checklist:

Indicate the SEMVER impact of the PR:

Major (when you make incompatible API changes)
Minor (when you add functionality in a backward compatible manner)
Patch (when you make backward compatible bug fixes)

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

To be included in the release note for any feature that is made obsolete/breaking

Give a brief explanation note about:

what was the old feature that was made obsolete

any replacement feature (if any), and

how the author should modify his website to migrate from the old feature to the replacement feature (if possible).

* Decouple monolithic Site god class. * Organize classes in Site/index.ts for git history, tracking before evict to separate files. * Extract responsibilities into dedicated manager classes - `SiteAssetsManager` - `SitePagesManager` (handling page collection and generation logic) - `SiteDeployManager` - `SiteGenerationManager` (orchestrating the build process) - Refactor `Site` class as Facade, delegate logic to underlying managers - Maintain public API compatibility for now.

codecov · 2026-01-14T06:52:27Z

Codecov Report

❌ Patch coverage is 72.17497% with 229 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.67%. Comparing base (2400af3) to head (8d1ca19).
⚠️ Report is 24 commits behind head on master.

Files with missing lines	Patch %	Lines
packages/core/src/Site/SiteGenerationManager.ts	59.19%	182 Missing ⚠️
packages/core/src/Site/SitePagesManager.ts	74.54%	27 Missing and 1 partial ⚠️
packages/core/src/Site/SiteDeployManager.ts	89.81%	11 Missing ⚠️
packages/core/src/Site/index.ts	86.04%	6 Missing ⚠️
packages/core/src/Site/SiteAssetsManager.ts	99.03%	1 Missing ⚠️
packages/core/src/Site/SiteConfig.ts	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2775      +/-   ##
==========================================
+ Coverage   62.30%   71.67%   +9.37%     
==========================================
  Files         130      134       +4     
  Lines        7184     7284     +100     
  Branches     1580     1514      -66     
==========================================
+ Hits         4476     5221     +745     
+ Misses       2644     2017     -627     
+ Partials       64       46      -18

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

This PR refactors the monolithic Site class into a facade pattern with specialized manager classes, improving code maintainability and separation of concerns without breaking the public API.

Changes:

Introduced four new manager classes to handle distinct responsibilities: SiteAssetsManager, SitePagesManager, SiteDeployManager, and SiteGenerationManager
Refactored the Site class as a facade that delegates to these managers while maintaining backward compatibility
Moved constants to a centralized location and reorganized test files to match the new structure

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
packages/core/src/Site/index.ts	Refactored Site class as facade delegating to manager classes
packages/core/src/Site/SitePagesManager.ts	New manager handling page creation and collection logic
packages/core/src/Site/SiteGenerationManager.ts	New manager orchestrating build process and page generation
packages/core/src/Site/SiteDeployManager.ts	New manager handling deployment operations
packages/core/src/Site/SiteAssetsManager.ts	New manager handling asset operations (CSS, JS, fonts, images)
packages/core/src/Site/constants.ts	Added new constants extracted from Site class
packages/core/test/unit/Site.test.ts	Removed old monolithic test file
packages/core/test/unit/Site/Site.test.ts	New test file for refactored Site facade
packages/core/test/unit/Site/SitePagesManager.test.ts	New test file for SitePagesManager
packages/core/test/unit/Site/SiteGenerationManager.test.ts	New test file for SiteGenerationManager
packages/core/test/unit/Site/SiteDeployManager.test.ts	New test file for SiteDeployManager
packages/core/test/unit/Site/SiteAssetsManager.test.ts	New test file for SiteAssetsManager

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

packages/core/src/Site/SiteGenerationManager.ts

packages/core/src/Site/index.ts

packages/core/src/Site/SitePagesManager.ts

- Removed internal and unused delegation methods from Site class. - Methods delegated without being used by CLI or external consumers removed. - Properties representing internal states (getters) removed as they break encapsulation and are unused. - This enforces the Facade pattern by hiding implementation details  and ensure Site object does not act as "God Class" exposing internal helpers. - Consumers can now clearly see the key interfaces intended for public use. - Removed associated unit tests and Jest mocks for the deleted facade methods in testcases

When style.codeTheme was set to a value that is not dark or light, it would cause a crash Added validation else set to default.

Encapsulate PagesManager baseUrlMap assign with a setter Standardize AssetsManager.copyTheme to return resolved promise

Caused by conflict of Jest reporter and lerna output streaming. Add --ci flag to jest command to fix

Update `SitePagesManager` test mock with `baseUrlMap` Fix log message spacing.

yihao03

Very welcomed changes to refactor the Site class as it has proven to be difficult to understand when I was studying the codebase.

yihao03 · 2026-01-16T07:01:03Z

packages/core/src/Site/SiteAssetsManager.ts

+   * Copies core-web bundles and external assets to the assets output folder
+   */
+  copyCoreWebAsset() {
+    const coreWebRootPath = path.dirname(require.resolve('@markbind/core-web/package.json'));


would this be a good opportunity to refactor require.resolve to use ESM alternatives? this applies to other instances of require.resolve() too

Yeap, that would be ideal to tackle in the next few PRs

yihao03 · 2026-01-16T07:12:07Z

packages/core/src/Site/SiteDeployManager.ts

+  message: string,
+  repo: string,
+  remote: string,
+  user?: { name: string; email: string; },


Is there already a user type defined somewhere? should we define it in a types.ts or a types/user.ts assuming that this type will show up often?

it seems that the defined type is only used in this specific component's logic, especially since the cli package which interfaces with core is also not in TS and has no type annotations

The types are not consolidated together probably because it was infeasible to do so when past developers were working to migrate the core file by file, but now that core has almost entirely been migrated to TS, we can definitely consider some form of consolidation of types, how to organize them, pros and cons.

Feel free to raise an issue if you have any good ideas / to facilitate discussion!

yihao03 · 2026-01-16T07:15:55Z

packages/core/src/Site/SiteDeployManager.ts

+    if (ciTokenVar) {
+      const ciToken = _.isBoolean(ciTokenVar) ? 'GITHUB_TOKEN' : ciTokenVar;
+      if (!process.env[ciToken]) {
+        throw new Error(`The environment variable ${ciToken} does not exist.`);
+      }
+      const githubToken = process.env[ciToken];
+      let repoSlug;
+
+      if (process.env.TRAVIS) {
+        repoSlug = SiteDeployManager.extractRepoSlug(options.repo, process.env.TRAVIS_REPO_SLUG);
+
+        options.user = {
+          name: 'Deployment Bot',
+          email: 'deploy@travis-ci.org',
+        };
+      } else if (process.env.APPVEYOR) {
+        repoSlug = SiteDeployManager.extractRepoSlug(options.repo, process.env.APPVEYOR_REPO_NAME);
+
+        options.user = {
+          name: 'AppVeyorBot',
+          email: 'deploy@appveyor.com',
+        };
+      } else if (process.env.GITHUB_ACTIONS) {
+        // Set cache folder to a location Github Actions can find.
+        process.env.CACHE_DIR = path.join(process.env.GITHUB_WORKSPACE || '.cache');
+        repoSlug = SiteDeployManager.extractRepoSlug(options.repo, process.env.GITHUB_REPOSITORY);
+
+        options.user = {
+          name: 'github-actions',
+          email: 'github-actions@github.com',
+        };
+      } else if (process.env.CIRCLECI) {
+        repoSlug = SiteDeployManager.extractRepoSlug(
+          options.repo,
+          `${process.env.CIRCLE_PROJECT_USERNAME}/${process.env.CIRCLE_PROJECT_REPONAME}`,
+        );
+
+        options.user = {
+          name: 'circleci-bot',
+          email: 'deploy@circleci.com',
+        };
+      } else {
+        throw new Error('-c/--ci should only be run in CI environments.');
+      }


perhaps can use switch case?

good idea, can raise a PR for this

gerteck added 4 commits January 14, 2026 15:28

Rename dev to isDevMode for better naming

fea1102

Add SiteAssetsManager class in own file

ab5db8c

Shift Site testcase to /Site

9a22f73

Add SiteAssetsManager testcases

3f72158

gerteck force-pushed the refactor/coreSiteIndex branch from 5c66e94 to 3f72158 Compare January 14, 2026 09:03

gerteck added 6 commits January 14, 2026 18:12

Shift SitePagesManager to separate file

98a5c45

Add SitePagesManager testcases

eab0d2d

Shift SiteDeployManager to separate file

afb2630

Add SiteDeployManager testcases

5e317e9

Shift SiteGenerationManager to separate file

5872fa8

Add SiteGenerationManager testcases

06c7970

gerteck requested a review from Copilot January 14, 2026 11:32

gerteck changed the title ~~Refactor structurally Site class into Facade and Managers~~ Structurally refactor Site class into Facade and separate Managers Jan 14, 2026

Copilot AI reviewed Jan 14, 2026

View reviewed changes

gerteck added 2 commits January 14, 2026 21:36

Fix slight lints and nits

60cf466

gerteck requested a review from Copilot January 14, 2026 14:38

Copilot started reviewing on behalf of gerteck January 14, 2026 14:39 View session

This comment was marked as outdated.

Sign in to view

gerteck added 2 commits January 15, 2026 19:36

Clean up testcase comments

cb0bdec

Add basic Site main interface functional tests

d1e6040

gerteck requested a review from Copilot January 15, 2026 11:37

Copilot started reviewing on behalf of gerteck January 15, 2026 11:38 View session

This comment was marked as outdated.

Sign in to view

gerteck added 4 commits January 15, 2026 23:14

Fix bug: Restore interfaces used in cli package from Site object

a1c8dda

Fix site config codeTheme style validation

fcd8cd9

When style.codeTheme was set to a value that is not dark or light, it would cause a crash Added validation else set to default.

Update logger info message with space

1b32557

Add core package Site functional tests

757a862

Refactor code smells

f61f672

Encapsulate PagesManager baseUrlMap assign with a setter Standardize AssetsManager.copyTheme to return resolved promise

gerteck requested a review from Copilot January 15, 2026 17:52

Copilot started reviewing on behalf of gerteck January 15, 2026 17:53 View session

This comment was marked as outdated.

Sign in to view

gerteck added 3 commits January 16, 2026 02:05

Fix malformed newlined test output

abcd454

Caused by conflict of Jest reporter and lerna output streaming. Add --ci flag to jest command to fix

Update testcase and console logging

fd1a7e6

Update `SitePagesManager` test mock with `baseUrlMap` Fix log message spacing.

Revert package.json changes

8d1ca19

MoshiMoshiMochi mentioned this pull request Jan 16, 2026

Add example of content processing flow in developer documentation #2777

Merged

14 tasks

yihao03 reviewed Jan 16, 2026

View reviewed changes

gerteck merged commit 277bab1 into MarkBind:master Jan 16, 2026
12 of 13 checks passed

github-actions bot added the r.Patch Version resolver: increment by 0.0.1 label Jan 16, 2026

Structurally refactor Site class into Facade and separate Managers #2775

Structurally refactor Site class into Facade and separate Managers #2775

Uh oh!

Conversation

gerteck commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

yihao03 left a comment

Choose a reason for hiding this comment

Uh oh!

yihao03 Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

gerteck Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yihao03 Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

gerteck Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

yihao03 Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

gerteck Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gerteck commented Jan 14, 2026 •

edited

Loading

codecov bot commented Jan 14, 2026 •

edited

Loading

gerteck Jan 16, 2026 •

edited

Loading