Skip to content

fix: preserve stop-node paths in updateTag callbacks - #886

Draft
vhinayindia wants to merge 1 commit into
NaturalIntelligence:masterfrom
vhinayindia:fix-stopnode-update-tag-path-20261006
Draft

vhinayindia wants to merge 1 commit into
NaturalIntelligence:masterfrom
vhinayindia:fix-stopnode-update-tag-path-20261006

Conversation

@vhinayindia

Copy link
Copy Markdown

Purpose / Goal

Keep the current stop node in the path matcher until updateTag has run. It is currently popped first, so the callback receives the parent's path. With jPath: false, matcher predicates also inspect the parent instead of the stop node. Path-based skipping and renaming therefore fail when combined with stopNodes.

No existing issue describes this exact case, so here is a reproduction as requested by the PR template:

const parser = new XMLParser({
  stopNodes: ['root.skip'],
  updateTag: (tagName, jPath) => jPath === 'root.skip' ? false : tagName
});
parser.parse('<root><skip><nested/></skip><keep>ok</keep></root>');

Actual: { root: { skip: '<nested/>', keep: 'ok' } }

Expected: { root: { keep: 'ok' } }

The fix moves addChild, which invokes updateTag, before the matcher pop. It leaves the raw stop-node content and subsequent sibling paths unchanged.

Tests

  • Four new regression cases fail on the unchanged source and pass with this fix.
  • Coverage includes path-based skipping/renaming in both output modes, self-closing and unpaired stop nodes, matcher attribute predicates with jPath: false, and following siblings.
  • npm test: 336 specs, 0 failures, 2 existing pending specs on Node 24.19.0.
  • Full Jasmine suite also passes on Node 14 and Node 22: 336 specs, 0 failures, 2 existing pending specs.
  • npm run test-types: passes.
  • git diff --check: passes.
  • npm run lint is blocked on both baseline and final source because benchmark/**/*.js matches no files. Focused ESLint with ECMAScript 2022 reports only the same existing unused variables in the two files.

Performance

Ran the documented node benchmark/XmlParser.mjs before and after on the same shared VM. Requests/second in the consecutive comparison:

Parser Before After
fxp 33,757 34,356
fxp, preserve order 40,447 41,447

These are noisy single-run observations, not a performance claim. The default benchmark does not use stop nodes; this change only reorders the existing callback and matcher pop in the stop-node branch.

Type

  • Bug Fix
  • Refactoring / Technology upgrade
  • New Feature

This branch has not been deployed

No deployments
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.

1 participant