repl: keep entries added while history file is loading

Project: nodejs/node | Language: JavaScript | Concepts: data-structures, error-handling, testing-strategy | Difficulty: mid
Link: https://github.com/nodejs/node/pull/64513

The problem

When using the Node.js REPL with a history file, lines can be evaluated before the file finishes loading (especially with non-standard input streams). Previously, any commands entered during this loading period were lost: once the file finished loading, its contents would overwrite the in-memory history, discarding anything entered in the meantime.

Why it's non-trivial

A naive fix might simply load the file and set the in-memory history to its contents, but this would still overwrite any new entries added during loading. The solution must merge the loaded history with any commands already entered, preserve order (newest first), and enforce the history size limit.

The approach

1. Parse loaded history, but do not overwrite in-memory history if it already contains new entries.

-      if (data) {
-        this[kHistory] = RegExpPrototypeSymbolSplit(/\r?\n+/, data, this[kSize]);
+      const loadedHistory = data ?
+        RegExpPrototypeSymbolSplit(/\r?\n+/, data, this[kSize]) :
+        [];

This creates a loadedHistory array from the file, or an empty array if the file is empty.

2. If in-memory history already has entries (i.e., commands were entered while loading), append the loaded entries to it, instead of overwriting.

+      // Lines can be evaluated while the history file is still being read,
+      // e.g. when the input stream does not support pausing. Such entries
+      // are already in the in-memory history (newest first), so append the
+      // persisted entries to them instead of discarding them.
+      if (this[kHistory].length > 0) {
+        ArrayPrototypePushApply(this[kHistory], loadedHistory);
+        if (this[kHistory].length > this[kSize]) {
+          ArrayPrototypeSplice(this[kHistory], this[kSize]);
+        }
+      } else {
+        this[kHistory] = loadedHistory;
+      }

If there are new in-memory entries, append the loaded entries. If the combined history exceeds the configured limit, trim it. If not, just use the loaded history.

3. Add a regression test to ensure this behavior is preserved.

+const r = repl.start({
+  input,
+  output,
+  prompt: '',
+  terminal: true,
+  useColors: false,
+});
+
+r.setupHistory(historyPath, common.mustSucceed(() => {
+  // The lines evaluated while the history file was being read must be kept,
+  // newest first, followed by the persisted entries.
+  assert.deepStrictEqual(
+    r.history,
+    ['const b = 2', 'const a = 1', 'persisted entry'],
+  );
+  assert.strictEqual(
+    fs.readFileSync(historyPath, 'utf8'),
+    'const b = 2\nconst a = 1\npersisted entry',
+  );
+  r.close();
+}));
+
+// Evaluated synchronously, before the history file has been read.
+input.emit('data', 'const a = 1\nconst b = 2\n');

This test simulates entering commands before the history file loads, and asserts that both the in-memory and persisted histories are merged as intended.

From the review thread

trivikr: node-test-commit-linux-containered CI is failing on ubuntu2404_sharedlibs_withoutssl_x64

Error [ERR_INSPECTOR_NOT_AVAILABLE]: Inspector is not available
...

mhayk: @trivikr Thanks for the report! Fixed in 59aa7e7 — the test now calls common.skipIfInspectorDisabled() like the other REPL history tests, since the REPL eval depends on the inspector and it is not available on --without-ssl builds.

Takeaways

• When asynchronously loading persistent state, always merge with any new in-memory changes that may have occurred during the load, rather than blindly overwriting.
• Write regression tests for race conditions and edge cases, especially when dealing with asynchronous file I/O and user input.
• Consider platform and environment differences (e.g., inspector availability, stream behavior) when writing tests for core modules.