Skip to content

feat(stax-xml): add synchronous cursor reader API#3

Open
Clickin wants to merge 2 commits intomasterfrom
codex/add-cursor-mode-support-in-stax-xml
Open

feat(stax-xml): add synchronous cursor reader API#3
Clickin wants to merge 2 commits intomasterfrom
codex/add-cursor-mode-support-in-stax-xml

Conversation

@Clickin
Copy link
Copy Markdown
Owner

@Clickin Clickin commented Mar 3, 2026

Motivation

  • Provide a cursor-style, synchronous pull API for callers that prefer indexed/name attribute access and explicit read()/hasNext() control over the existing iterable parser.
  • Reuse the fast existing StaxXmlParserSync implementation to avoid changing existing parser constructors or behavior.

Description

  • Added a new cursor reader implementation StaxXmlCursorReaderSync with a factory createStaxXmlCursorReaderSync that wraps StaxXmlParserSync and exposes hasNext(), read(), getEvent(), and requireEvent() methods (packages/stax-xml/src/StaxXmlCursorReaderSync.ts).
  • Added XmlCursorEvent and internal AttrStore modules to provide cursor-friendly accessors for event fields and attributes (packages/stax-xml/src/XmlCursorEvent.ts, packages/stax-xml/src/AttrStore.ts).
  • Introduced cursor-specific types without modifying existing event interfaces: CursorXmlEventType, CursorAttribute, XmlCursorEventLike, and XmlCursorReaderSyncLike (packages/stax-xml/src/types.ts).
  • Exported the new cursor reader, cursor event, and cursor types from the package entry file while preserving all existing exports (packages/stax-xml/src/index.ts).
  • Added tests for the cursor reader behavior and a small additive README example demonstrating cursor usage (packages/stax-xml/test/cursor-reader-sync.test.ts, README.md, packages/stax-xml/README.md).

Testing

  • Type-check: ran tsc -p packages/stax-xml/tsconfig.json --noEmit, which completed successfully. ✅
  • Unit tests: ran vitest for the new cursor tests via ./node_modules/.bin/vitest run packages/stax-xml/test/cursor-reader-sync.test.ts, which passed (2 tests). ✅
  • Existing parser tests: ran ./node_modules/.bin/vitest run packages/stax-xml/test/parser-sync.test.ts, which passed (29 tests). ✅
  • Note: attempting pnpm --filter=stax-xml test failed due to environment network/corepack fetch error; this is unrelated to the code changes. ⚠️

Codex Task

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 100f47e99f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +44 to +46
name,
localName: typedInfo.localName,
prefix: typedInfo.prefix,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Derive cursor attribute fields from qualified name

When AttrStore.fromEvent reads event.attributesWithPrefix, it assumes each value already contains localName and that the entry key is the full attribute name. For StartElementEvents produced by the existing async parser, the map is keyed by local name and values do not include localName, so XmlCursorEvent ends up with CursorAttribute.localName as undefined and drops prefixes from name (for example, a:id becomes id), which also breaks getAttributeValue('a:id') lookups when wrapping those events.

Useful? React with 👍 / 👎.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant