Skip to content

fix: preserve empty-string parents in JSON roundtrips - #247

Merged
rustedgrail merged 2 commits into
dagrejs:masterfrom
dajiaohuang:fix/json-empty-parent-20261015
Oct 23, 2026
Merged

rustedgrail merged 2 commits into
dagrejs:masterfrom
dajiaohuang:fix/json-empty-parent-20261015

Conversation

@dajiaohuang

Copy link
Copy Markdown
Contributor

Problem

An empty string is a node ID accepted by the Graph API, but JSON restoration silently drops it when it is used as a parent:

const { Graph, json } = require('@dagrejs/graphlib');
const g = new Graph({ compound: true }).setParent('child', '');
const restored = json.read(JSON.parse(JSON.stringify(json.write(g))));

console.log(g.parent('child'));        // ''
console.log(restored.parent('child')); // undefined (before this change)
console.log(restored.children(''));    // [] (before this change)

The serialized object contains parent: '', but json.read treats it as an absent parent. This moves the child to the root instead of restoring the original hierarchy.

Change

Test the optional parent field against undefined, matching json.write and Graph.setParent, rather than testing its truthiness. Add a regression covering the parent, its children and the root children after a stringify/parse roundtrip. No graph API or serialization format change.

Validation

  • Regression fails on master d649cb32a827619da09959c89d8973aaf2370b4e and passes with this change.
  • npm ci, npm run lint, npm run build, npm run typecheck: pass.
  • npm test -- --runInBand: 258/259 tests pass (19/20 suites). The unchanged dist-exports.test.ts ESM smoke test fails on Windows because it imports a raw D: path rather than a file: URL (ERR_UNSUPPORTED_ESM_URL_SCHEME). This is separate from JSON restoration.
  • Fresh packed-package consumer: CJS and native ESM graph operations, compound multigraph JSON roundtrips (including the regression), and browser-bundle VM smoke checks pass.

Local environment: Windows, Node 24.14.1, TypeScript 5.9.3. make is unavailable; the npm commands used by its build/test/lint targets were run directly. Linux/Node 18 CI has not been run locally. Generated bundles were rebuilt for validation; only canonical source and the regression test are committed, following the existing source-fix/release-build convention.

@rustedgrail
rustedgrail merged commit 233ce10 into dagrejs:master Oct 23, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants