refactor: use parseXml instead of direct call to DOMParser by tbouffard · Pull Request #991 · maxGraph/maxGraph

tbouffard · 2026-01-07T14:43:02Z

Use the existing function to remove duplication and make the code easier to read.

Summary by CodeRabbit

Refactor
- Consolidated XML parsing to a centralized utility across internal rendering paths for more consistent document handling.
Documentation
- Updated docs and examples to reference current utilities and API terminology.
Style
- Simplified conditional checks and modernized variable declarations for clearer, more maintainable code.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Use the existing function to remove duplication and make the code easier to read.

coderabbitai · 2026-01-07T14:43:30Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Replaces direct DOMParser usage with a centralized xmlUtils.parseXml call in XML/HTML parsing paths and updates related imports and minor docs/syntax in two files. No public APIs or method signatures changed.

Changes

Cohort / File(s)	Summary
XML parsing updates `packages/core/src/util/MaxXmlRequest.ts`, `packages/core/src/view/canvas/SvgCanvas2D.ts`	Replaced inline `DOMParser().parseFromString` usage with `xmlUtils.parseXml(...)`. `SvgCanvas2D` imports extended to include `parseXml`; parsing code paths (e.g., `convertHtml`, `createDiv`) updated to use `parseXml` and access `documentElement`.
Docs & minor syntax `packages/core/src/view/canvas/SvgCanvas2D.ts`	JSDoc references updated from `mxUtils/mxConstants` to `xmlUtils/constants`; some `var` → `const` modernizations and simplified conditional checks (e.g., `== null` → `!`). No public API changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is very brief and lacks required template sections including issue reference, scope confirmation, testing details, documentation changes, and conventional commits compliance.	Add required sections: issue number reference (closes #xxx), scope confirmation, test details, documentation notes, and verify the title follows Conventional Commits format.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main refactoring change: replacing direct DOMParser calls with the centralized parseXml utility function across multiple files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In @packages/core/src/util/MaxXmlRequest.ts:
- Line 20: The import in MaxXmlRequest.ts is missing the .js extension; update
the import of parseXml from './xmlUtils' to include the file extension (i.e.,
'./xmlUtils.js') so it satisfies the n/file-extension-in-import ESLint rule and
keeps the parseXml import correct.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 42384ac and 07ac543.

📒 Files selected for processing (2)

packages/core/src/util/MaxXmlRequest.ts
packages/core/src/view/canvas/SvgCanvas2D.ts

🧰 Additional context used

📓 Path-based instructions (6)

packages/core/src/**/*.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

packages/core/src/**/*.ts: Include .js file extensions in all import statements within packages/core/src/ to comply with ESLint rule n/file-extension-in-import.
Do not use console statements in any code. Use proper logging mechanisms instead (ESLint rule: no-console: error).
Do not use eval() function in any code (ESLint rule: no-eval: error).
Do not use const enums. Convert all const enums to regular enums (ESLint rule forbids const enums).
TypeScript source files in core package must strictly maintain typing with no implicit types and all functions having explicit return type annotations.

Use singular form for folder names (e.g., handler/ not handlers/) and kebab-case for folder names (e.g., my-feature/ not myFeature/ or my_feature/)

Files:

packages/core/src/util/MaxXmlRequest.ts
packages/core/src/view/canvas/SvgCanvas2D.ts

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use single quotes in TypeScript/JavaScript code (Prettier config: singleQuote: true).

Files:

packages/core/src/util/MaxXmlRequest.ts
packages/core/src/view/canvas/SvgCanvas2D.ts

**/*.{ts,tsx,js,jsx,json,md}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Format code using Prettier with tab width 2, trailing comma ES5, print width 90, and end of line auto.

Files:

packages/core/src/util/MaxXmlRequest.ts
packages/core/src/view/canvas/SvgCanvas2D.ts

**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.ts: Use the isNullish() function when checking for null or undefined on variables that can have falsy values (numbers, strings, booleans) to avoid bugs where 0, "", or false are incorrectly treated as nullish
Use the log() function from internal/utils.js instead of accessing GlobalConfig.logger directly for logging
Use functions translate() and isI18nEnabled() from internal/i18n-utils.js instead of accessing GlobalConfig.i18n directly for internationalization
Always wrap multiple model changes in batchUpdate() to ensure efficient graph updates

Files:

packages/core/src/util/MaxXmlRequest.ts
packages/core/src/view/canvas/SvgCanvas2D.ts

**/*.{ts,js}

📄 CodeRabbit inference engine (CLAUDE.md)

Prefer using BaseGraph in production with explicit feature registration for optimal tree-shaking and minimal bundle size; use Graph only for prototyping and demos with automatic defaults

Files:

packages/core/src/util/MaxXmlRequest.ts
packages/core/src/view/canvas/SvgCanvas2D.ts

packages/core/src/**

📄 CodeRabbit inference engine (CLAUDE.md)

Organize code using the mixin pattern where each mixin provides a specific capability with two files: implementation (.ts) and type definitions (.type.ts), with unified type definitions in _graph-mixins-types.ts

Files:

packages/core/src/util/MaxXmlRequest.ts
packages/core/src/view/canvas/SvgCanvas2D.ts

🧠 Learnings (1)

📓 Common learnings

Learnt from: tbouffard
Repo: maxGraph/maxGraph PR: 857
File: packages/core/src/view/plugins/SelectionHandler.ts:1081-1092
Timestamp: 2025-06-26T13:27:19.012Z
Learning: User tbouffard prefers to keep PRs focused on their main objective and defer implementation improvements or logic fixes to future dedicated PRs. For maxGraph, refactoring PRs like Dictionary→Map migration should not include unrelated logic improvements even when valid issues are identified in the migrated code.

Learnt from: tbouffard
Repo: maxGraph/maxGraph PR: 849
File: packages/html/stories/DragSource.stories.js:98-101
Timestamp: 2025-06-13T07:48:10.300Z
Learning: User tbouffard prefers answers in English; avoid switching to other languages in future replies.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

GitHub Check: build
GitHub Check: build (ubuntu-24.04)
GitHub Check: build (windows-2022)

🔇 Additional comments (5)

packages/core/src/util/MaxXmlRequest.ts (1)

217-217: LGTM! Successful refactoring to centralized XML parsing.

The change from direct DOMParser usage to the parseXml utility function achieves the PR objective of removing duplication and improving code readability.

packages/core/src/view/canvas/SvgCanvas2D.ts (4)

34-34: LGTM! Import statement correctly includes .js extension.

The import properly extends the existing xmlUtils import to include parseXml and follows the coding guideline requiring .js file extensions.

61-80: LGTM! Documentation examples modernized and aligned with current conventions.

The JSDoc example updates improve code quality by:

Referencing current module naming (xmlUtils, constants)

Modernizing variable declarations (const instead of var)

Simplifying conditional checks

These documentation improvements complement the core refactoring objective.

1090-1090: LGTM! HTML parsing centralized through parseXml utility.

The refactoring successfully replaces direct DOMParser usage with the centralized parseXml function, achieving the PR objective for the convertHtml method.

1150-1150: LGTM! XML parsing centralized through parseXml utility.

The refactoring successfully replaces direct DOMParser usage with the centralized parseXml function, achieving the PR objective for the createDiv method.

… used in Editor)

tbouffard · 2026-01-07T15:35:15Z

Converted to draft, the changes introduce a circular dependency between xmlUtils and MaxXmlRequest.

This is due to the getViewXml function, but it is not a function manipulating generic XML content, but is generating XML with Codec. It is also only used in the Editor class, so it should be moved first to another file.
See #992

… CHANGE)" This reverts commit 44c446d.

…on, only used in Editor)" This reverts commit 2609a8f.

sonarqubecloud · 2026-01-09T09:50:03Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

refactor: use parseXml instead of direct call to DOMParser

07ac543

Use the existing function to remove duplication and make the code easier to read.

tbouffard added the refactor Code refactoring label Jan 7, 2026

coderabbitai Bot reviewed Jan 7, 2026

View reviewed changes

Comment thread packages/core/src/util/MaxXmlRequest.ts Outdated

tbouffard added 2 commits January 7, 2026 16:10

fix lint errors

a532419

EXTRA WIP move getViewXml out of xmlUtils (non related function, only…

2609a8f

… used in Editor)

tbouffard marked this pull request as draft January 7, 2026 15:35

tbouffard added 4 commits January 7, 2026 16:44

EXTRA WIP move getViewXml to xmlViewUtils (this is a BREAKING CHANGE)

44c446d

Revert "EXTRA WIP move getViewXml to xmlViewUtils (this is a BREAKING…

79c8a9b

… CHANGE)" This reverts commit 44c446d.

Revert "EXTRA WIP move getViewXml out of xmlUtils (non related functi…

e529ad8

…on, only used in Editor)" This reverts commit 2609a8f.

Merge branch 'main' into refactor/use_xmlutils_parseString

ce79773

tbouffard marked this pull request as ready for review January 9, 2026 10:09

tbouffard merged commit 2f4bb07 into main Jan 9, 2026
12 checks passed

tbouffard deleted the refactor/use_xmlutils_parseString branch January 9, 2026 10:10

coderabbitai Bot mentioned this pull request Feb 5, 2026

refactor: improve type guard for domUtils.isNode #1000

Merged

coderabbitai Bot mentioned this pull request Feb 25, 2026

fix: parse HTML instead of XML in SvgCanvas2D.convertHtml #1012

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: use parseXml instead of direct call to DOMParser#991

refactor: use parseXml instead of direct call to DOMParser#991
tbouffard merged 7 commits into
mainfrom
refactor/use_xmlutils_parseString

tbouffard commented Jan 7, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jan 7, 2026 •

edited

Loading

Review skipped

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

tbouffard commented Jan 7, 2026 •

edited

Loading

Uh oh!

sonarqubecloud Bot commented Jan 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

tbouffard commented Jan 7, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tbouffard commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud Bot commented Jan 9, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

tbouffard commented Jan 7, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jan 7, 2026 •

edited

Loading

tbouffard commented Jan 7, 2026 •

edited

Loading