Skip to content

refactor(docker): split docker.ts into modules - #1093

Merged
skjnldsv merged 1 commit into
mainfrom
refactor/split-docker-module
Sep 4, 2026
Merged

skjnldsv merged 1 commit into
mainfrom
refactor/split-docker-module

Conversation

@skjnldsv

@skjnldsv skjnldsv commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

lib/docker.ts had grown to 825 lines and mixed container lifecycle, command
execution, app installation, the Composer bootstrap, log extraction, snapshots
and user creation. This splits it into one module per concern under
lib/docker/, with lib/docker/index.ts re-exporting the same 20 public names.

No behaviour change, it is just a move.

This change was AI assisted.

`lib/docker.ts` had grown to 825 lines covering container lifecycle,
command execution, app installation, the Composer bootstrap, log
extraction, snapshots and user creation. Split it into one module per
concern under `lib/docker/`, with `lib/docker/index.ts` re-exporting the
same 20 public names so the `@nextcloud/e2e-test-server/docker` subpath
and the built entry names are unchanged.

This is a pure move: no behaviour change. Three deltas were required to
break the module cycle and are the only edits to the moved code:

* the `_serverBranch` module state moved to `client.ts` behind
  `getServerBranch()`/`setServerBranch()`, since `lifecycle.ts` writes it
  and `configure.ts` reads it
* `pathExists`, `sleep`, `asNodeStream`, `pruneAppsWritableVolume` and
  `APPS_WRITABLE_VOLUME` gained `export` to stay reachable across
  modules; they are not re-exported from the barrel and remain private
* `getContainerIP` got an explicit `Container` annotation on its
  defaulted parameter

`package.json` needed the `./docker` types path repointed to
`./dist/docker/index.d.ts`; the previous `./dist/docker.d.ts` is no
longer emitted, which would otherwise have left TypeScript consumers of
that subpath without types.

Verified with `ts:check`, `lint`, `build` (dist entry names and the
runtime/type export sets are identical), and both node integration
specs: 13/13 pass.

Assisted-by: ClaudeCode:claude-sonnet-5
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv skjnldsv added 3. to review Waiting for reviews enhancement New feature request AI assisted This PR contains AI-assisted commits labels Sep 4, 2026
@skjnldsv skjnldsv self-assigned this Sep 4, 2026
@skjnldsv
skjnldsv requested a review from susnux September 4, 2026 11:11
@skjnldsv
skjnldsv enabled auto-merge September 4, 2026 11:12

@susnux susnux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH not sure I find this more maintainable 😅

@skjnldsv
skjnldsv merged commit ea48612 into main Sep 4, 2026
11 checks passed
@skjnldsv
skjnldsv deleted the refactor/split-docker-module branch September 4, 2026 12:00
@skjnldsv

skjnldsv commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

TBH not sure I find this more maintainable 😅

Really?? 😅😅😅
For me 800+ lines is very close to a monolith. 🫣

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews AI assisted This PR contains AI-assisted commits enhancement New feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants