refactor(git-sync): address PR #119 review #3 — honest gitRemote scaffolding comments, env example, shared ESM bridge
1. gitRemote is NOT yet consumed (the vendored engine has no remote-push path,
SPEC §7). Corrected the buildSettings docstring (it wrongly called gitRemote
"load-bearing") and marked the env -> validation -> getter -> buildSettings
chain as inert SCAFFOLDING for the deferred remote-push feature at all three
sites. Kept the wiring (harmless; removing only churns).
2. .env.example: document that GIT_SYNC_REMOTE_TEMPLATE substitutes the literal
"{spaceId}" per-space (with the example), so an operator doesn't point every
space at one remote.
3. Extracted the copy-pasted CJS->ESM dynamic-import bridge
(`new Function('s','return import(s)')`) into one shared
common/helpers/esm-import.ts; git-sync.loader, docmost-client.loader and
mcp.service now import it and keep their own typed loadX() wrappers.
Deferred (notes only, not implemented):
- lcs.ts + three-way-merge.ts could move into packages/git-sync, but that engine
is vendored (manual re-sync) — added a one-line note at three-way-merge.ts to
revisit once the re-sync story is settled.
- schema-core single source + BullMQ/fencing remain documented from prior rounds.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import { pathToFileURL } from 'node:url';
|
||||
import { esmImport } from '../../common/helpers/esm-import';
|
||||
import type {
|
||||
VaultGit as VaultGitClass,
|
||||
vaultGitEnv as vaultGitEnvFn,
|
||||
@@ -21,16 +22,9 @@ interface GitSyncModule {
|
||||
markdownToProseMirror: typeof markdownToProseMirrorFn;
|
||||
}
|
||||
|
||||
// TS with module:commonjs downlevels a literal `import()` to `require()`, which
|
||||
// cannot load the ESM-only `@docmost/git-sync` package. Indirect through
|
||||
// Function so the real dynamic `import()` survives compilation and can load ESM
|
||||
// from CommonJS at runtime (same trick as
|
||||
// apps/server/src/core/ai-chat/tools/docmost-client.loader.ts and
|
||||
// integrations/mcp/mcp.service.ts).
|
||||
const esmImport = new Function(
|
||||
'specifier',
|
||||
'return import(specifier)',
|
||||
) as (specifier: string) => Promise<unknown>;
|
||||
// The CJS->ESM dynamic-import bridge lives in one shared helper
|
||||
// (common/helpers/esm-import.ts); see it for why `import()` must be hidden from
|
||||
// the TS commonjs downleveler. The typed `loadGitSync()` wrapper stays here.
|
||||
|
||||
// Memoize the in-flight/loaded module so the dynamic import runs at most once.
|
||||
let modulePromise: Promise<GitSyncModule> | null = null;
|
||||
|
||||
@@ -111,10 +111,20 @@ export class GitSyncOrchestrator implements OnModuleInit, OnModuleDestroy {
|
||||
/**
|
||||
* Build the engine `Settings` for a space. The engine's REST-era fields
|
||||
* (docmostApiUrl/email/password) are unused on the native path — the
|
||||
* datasource writes in-process — so they are placeholders; only `vaultPath`,
|
||||
* `gitRemote`, and the tunables are load-bearing.
|
||||
* datasource writes in-process — so they are placeholders; only `vaultPath`
|
||||
* and the tunables are load-bearing today.
|
||||
*
|
||||
* `gitRemote` is NOT yet consumed: the vendored engine has no remote-push path
|
||||
* (see engine/git.ts, engine/pull.ts, SPEC §7 — remote push is deferred), so
|
||||
* the GIT_SYNC_REMOTE_TEMPLATE env -> validation -> getter -> this field chain
|
||||
* is inert SCAFFOLDING kept in place for the future remote-push feature. It is
|
||||
* harmless (the engine ignores it) and removing it would only churn; we still
|
||||
* populate it so the wiring is ready when the engine grows a push path.
|
||||
*/
|
||||
private async buildSettings(spaceId: string): Promise<Settings> {
|
||||
// Scaffolding for the deferred remote-push feature — the engine does not read
|
||||
// `gitRemote` yet (see the docstring above). Substitute {spaceId} per-space so
|
||||
// the value is correct the moment the engine starts consuming it.
|
||||
const remoteTemplate = this.environmentService.getGitSyncRemoteTemplate();
|
||||
const gitRemote = remoteTemplate
|
||||
? remoteTemplate.replace(/\{spaceId\}/g, spaceId)
|
||||
|
||||
@@ -17,6 +17,13 @@
|
||||
* the human and/or git rewrote; resolve each region three-way. Stable anchor
|
||||
* blocks are emitted from LIVE so the applier keeps the existing Yjs block
|
||||
* instances (and the human's in-flight edits) in place.
|
||||
*
|
||||
* LOCATION (deferred): this and its `lcs.ts` sibling are pure, framework-free and
|
||||
* could conceptually live in `packages/git-sync` (the engine). They are kept in
|
||||
* the server integration on purpose: `packages/git-sync` is a VENDORED engine
|
||||
* (pinned upstream, manually re-synced), so adding first-party files there
|
||||
* complicates the re-sync story, and the only consumer today is the server. Move
|
||||
* them into the engine only once the vendoring re-sync story is settled.
|
||||
*/
|
||||
|
||||
import { buildLcsTable } from './lcs';
|
||||
|
||||
Reference in New Issue
Block a user