Skip to content

Commit 6200614

Browse files
claudeclaude
committed
revert(prefix-source): drop buffer()/buffers() caching — unsafe with mutable child
PrefixSource wraps an arbitrary Source. ReplaceSource (.replace, .insert), ConcatSource (.add), and CompatSource over a mutable SourceLike all break the immutability assumption. source() reads the child on every call, so caching the derived buffer would silently go stale after a child mutation: const inner = new ReplaceSource(new RawSource("hello world")); const ps = new PrefixSource("> ", inner); ps.buffer(); // populates cache: "> hello world" inner.replace(6, 10, "you"); ps.source(); // "> hello you" -- correct ps.buffer(); // "> hello world" -- STALE (with caching) RawSource / OriginalSource / SourceMapSource can cache safely because they own their value. PrefixSource doesn't have that invariant. The amortized 10x speedup wasn't worth a correctness regression. Adds a regression test that calls source/buffer/buffers, mutates the child, and asserts all three reflect the new state. If a caller wants the speedup, wrap the PrefixSource in CachedSource — that's the documented opt-in cache. https://claude.ai/code/session_01EHhGq9PRFRGefVtwwasCqZ
1 parent 0ec6dbd commit 6200614

2 files changed

Lines changed: 47 additions & 59 deletions

File tree

‎lib/PrefixSource.js‎

Lines changed: 28 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,6 @@ const RawSource = require("./RawSource");
99
const Source = require("./Source");
1010
const { getMap, getSourceAndMap } = require("./helpers/getFromStreamChunks");
1111
const streamChunks = require("./helpers/streamChunks");
12-
const {
13-
isDualStringBufferCachingEnabled,
14-
} = require("./helpers/stringBufferUtils");
1512

1613
/** @typedef {import("./Source").HashLike} HashLike */
1714
/** @typedef {import("./Source").MapOptions} MapOptions */
@@ -46,16 +43,6 @@ class PrefixSource extends Source {
4643
typeof source === "string" || Buffer.isBuffer(source)
4744
? new RawSource(source, true)
4845
: source;
49-
/**
50-
* @private
51-
* @type {Buffer | undefined}
52-
*/
53-
this._cachedBuffer = undefined;
54-
/**
55-
* @private
56-
* @type {Buffer[] | undefined}
57-
*/
58-
this._cachedBuffers = undefined;
5946
}
6047

6148
getPrefix() {
@@ -75,62 +62,44 @@ class PrefixSource extends Source {
7562
return prefix + node.replace(REPLACE_REGEX, `\n${prefix}`);
7663
}
7764

78-
/**
79-
* Cache the encoded buffer so repeat `buffer()` / `size()` calls don't
80-
* re-run the regex replace and re-encode the prefixed string. Mirrors
81-
* the `_valueAsBuffer` caching used by `RawSource`, `OriginalSource`,
82-
* and `SourceMapSource`. First call still goes through `this.source()`
83-
* plus `Buffer.from` because V8's string-replace + utf8 encode beats
84-
* a hand-written single-pass copy on typical ASCII inputs.
85-
* @returns {Buffer} buffer
86-
*/
87-
buffer() {
88-
if (this._cachedBuffer !== undefined) return this._cachedBuffer;
89-
const value = Buffer.from(/** @type {string} */ (this.source()), "utf8");
90-
if (isDualStringBufferCachingEnabled()) {
91-
this._cachedBuffer = value;
92-
}
93-
return value;
94-
}
65+
// Note: we deliberately don't cache buffer() / buffers() / size() here.
66+
// `_source` can be mutable (ReplaceSource, ConcatSource, CompatSource
67+
// wrapping a SourceLike) and `source()` reads it on every call; caching
68+
// the derived result would silently go stale after a child mutation.
69+
// `RawSource` / `OriginalSource` / `SourceMapSource` cache their own
70+
// buffer because they're self-contained — that invariant doesn't hold
71+
// here. Wrap PrefixSource in CachedSource if you need the speedup.
9572

9673
/**
9774
* Returns a `Buffer[]` that concatenates to the prefixed source. Each
9875
* content chunk is a `subarray` of the underlying buffer (no copy);
99-
* only the prefix buffer is materialized. Cached for repeat calls.
76+
* only the prefix buffer is materialized. This skips the
77+
* [this.buffer()] wrap that the default Source.buffers() would do.
10078
* @returns {Buffer[]} buffers
10179
*/
10280
buffers() {
103-
if (this._cachedBuffers !== undefined) return this._cachedBuffers;
10481
const prefix = this._prefix;
105-
/** @type {Buffer[]} */
106-
let result;
10782
if (prefix.length === 0) {
108-
result = /** @type {Source} */ (this._source).buffers();
109-
} else {
110-
const content = this._source.buffer();
111-
const prefixBuffer = Buffer.from(prefix, "utf8");
112-
if (content.length === 0) {
113-
result = [prefixBuffer];
114-
} else {
115-
result = [prefixBuffer];
116-
const len = content.length;
117-
let i = 0;
118-
while (i < len) {
119-
const nl = content.indexOf(0x0a, i);
120-
if (nl === -1) {
121-
result.push(i === 0 ? content : content.subarray(i));
122-
break;
123-
}
124-
result.push(content.subarray(i, nl + 1));
125-
// Match the regex `/\n(?=.|\s)/g` — only re-emit the prefix
126-
// when at least one more byte follows the newline.
127-
if (nl + 1 < len) result.push(prefixBuffer);
128-
i = nl + 1;
129-
}
130-
}
83+
return /** @type {Source} */ (this._source).buffers();
13184
}
132-
if (isDualStringBufferCachingEnabled()) {
133-
this._cachedBuffers = result;
85+
const content = this._source.buffer();
86+
const prefixBuffer = Buffer.from(prefix, "utf8");
87+
if (content.length === 0) return [prefixBuffer];
88+
/** @type {Buffer[]} */
89+
const result = [prefixBuffer];
90+
const len = content.length;
91+
let i = 0;
92+
while (i < len) {
93+
const nl = content.indexOf(0x0a, i);
94+
if (nl === -1) {
95+
result.push(i === 0 ? content : content.subarray(i));
96+
return result;
97+
}
98+
result.push(content.subarray(i, nl + 1));
99+
// Match the regex `/\n(?=.|\s)/g` — only re-emit the prefix when
100+
// at least one more byte follows the newline.
101+
if (nl + 1 < len) result.push(prefixBuffer);
102+
i = nl + 1;
134103
}
135104
return result;
136105
}

‎test/PrefixSource.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const { PrefixSource } = require("../");
77
const { OriginalSource } = require("../");
88
const { ConcatSource } = require("../");
99
const { RawSource } = require("../");
10+
const { ReplaceSource } = require("../");
1011
const { withReadableMappings } = require("./helpers");
1112

1213
describe("prefixSource", () => {
@@ -210,4 +211,22 @@ describe("prefixSource", () => {
210211
expect(source.buffer().toString("utf8")).toBe("> héllo\n> wörld");
211212
expect(source.buffer().toString("utf8")).toBe(source.source());
212213
});
214+
215+
it("should reflect mutations to the underlying source on subsequent calls", () => {
216+
const inner = new ReplaceSource(new RawSource("hello world"));
217+
const source = new PrefixSource("> ", inner);
218+
expect(source.source()).toBe("> hello world");
219+
expect(source.buffer().toString("utf8")).toBe("> hello world");
220+
expect(Buffer.concat(source.buffers()).toString("utf8")).toBe(
221+
"> hello world",
222+
);
223+
224+
inner.replace(6, 10, "you");
225+
226+
expect(source.source()).toBe("> hello you");
227+
expect(source.buffer().toString("utf8")).toBe("> hello you");
228+
expect(Buffer.concat(source.buffers()).toString("utf8")).toBe(
229+
"> hello you",
230+
);
231+
});
213232
});

0 commit comments

Comments
 (0)