Skip to content

extract: destructive full-file overwrite instead of in-place region splice; aliased file= paths clobber each other #21

Description

@adrianbrowning

extract is destructive: it overwrites target files instead of splicing regions in place, and aliased paths clobber each other.

Version: mdcode-ts@0.0.4

Running mdcode extract README.md against a README whose blocks use file=… region=… destroyed 27 source files: 2292 lines deleted, 452 added. The files were real, working test files; the README blocks are only excerpts of them.

Root cause

In dist/commands/extract.js every branch ends in a bare full-file write:

// all three branches:
await writeFile(filePath, parts.join("\n").trim() + "\n", "utf-8");   // multi-region
await writeFile(filePath, content, "utf-8");                          // single region
await writeFile(filePath, items[0]?.block.code || "", "utf-8");       // no region

There is no read-existing-file-then-splice path. For a region= block the file is synthesized from scratch as // #region X + block body + // #endregion X.

The package already exports exactly the primitive needed and extract never calls it:

// dist/region.d.ts:15
export declare function replace(source: string, regionName: string, newContent: string): RegionReplaceResult;

update (the reverse direction) uses region-aware logic; extract does not.

Three separate failures

1. Surrounding code is destroyed

Source file before (38 lines):

import assert from "node:assert/strict";
import { describe, test } from "node:test";

import { prisma } from "#client";
import { expectSQL } from "../test-utils.ts";

describe("README Example: .$from basic", () => {
  test("should create basic from query", () => {
    const query =
    // #region example-$from
      prisma.$from("User");
    // #endregion

    const expectedSQL =
      // #region example-from
      "FROM User;";
      // #endregion example-from

    assert.equal(query.getSQL(), expectedSQL);
  });
  // … two more regions
});

README block:

```typescript file=./tests/readme/from-basic.ts region=example-$from
      prisma.$from("User");
```

After extract (3 lines — file is now not valid as a test, imports and assertions gone):

// #region example-$from
      prisma.$from("User");
// #endregion example-$from

2. Regions absent from the README are deleted

A consequence of (1): only regions that appear as README blocks survive. Regions that exist in the file but aren't mirrored in the docs are silently dropped.

file regions before README blocks regions after
select-advanced.ts 21 14 14
select-fns.ts 18 10 10
table-alias.ts 15 9 9
having.ts 12 8 8
pagination.ts 7 4 4
from-basic.ts 4 1 1

3. fileMap is keyed on the literal file= string, so aliased paths clobber

filePath = join(outputDir, block.meta.file);
if (!fileMap.has(filePath)) fileMap.set(filePath, []);

filePath is not resolved or normalised. If the same file is reachable via two spellings, it gets two fileMap entries, each written with a separate writeFile — last write wins and the earlier group is lost.

Real case: one file referenced two ways in the same README (the second path traverses a directory symlink to the first):

file=../../shared-tests/readme/where.ts          →  2 blocks
file=../usage-sqlite-v7/tests/readme/where.ts    → 29 blocks

Both resolve to the same file on disk. Result: 48 regions → 2, file truncated to 11 lines. 29 examples lost in one command.

This is the most damaging of the three because it discards blocks that were correctly declared in the README.

Expected behaviour

For a block with region=, when the target file already exists:

  1. Read the existing file.
  2. region.replace(source, regionName, block.code) for each block targeting it.
  3. Write the result.

Only synthesize a fresh file when the target does not exist. Full-file overwrite of an existing file arguably shouldn't happen at all without an explicit --force.

And key fileMap on realpathSync(filePath) (falling back to path.resolve when the file doesn't exist yet) so aliased paths merge into one entry instead of racing.

Not a bug, but worth a note

Anonymous blocks (no file=) are dumped as block-<N>.<ext> into the working directory — 20+ stray files including block-1.sh, block-4.json, block-198.sql. This is documented and --ignore-anonymous suppresses it. Possibly worth making the default, or at least mentioning in the extract help text, since it's surprising alongside a plain mdcode extract README.md.

Suggested guard

Given the blast radius, extract refusing to write when the resulting file would be smaller than the existing one — unless --force — would have caught all three failures here.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions