Skip to content

Externalize xmldom/xpath for ESM/CJS builds#12

Merged
edgarsj merged 3 commits into
mainfrom
fix/esm-node-imports
Mar 17, 2026
Merged

Externalize xmldom/xpath for ESM/CJS builds#12
edgarsj merged 3 commits into
mainfrom
fix/esm-node-imports

Conversation

@edgarsj

@edgarsj edgarsj commented Mar 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • Replace inline require() with top-level import for @xmldom/xmldom and xpath in xmlParser.ts
  • Mark both as external in Rollup ESM/CJS build config, so output uses proper import/require statements
  • Keep them bundled inline for UMD (browser) build via separate isExternalModuleUmd function
  • Stub out these Node-only deps in web-test-runner so browser tests don't try to load them
  • Also externalizes @peculiar/asn1-* packages (were being unnecessarily bundled)

Fixes "XML DOM parser not available" error when ESM consumers (projects with "type": "module") use edockit/trusted-list/build in Node.js.

Test plan

  • All 122 unit/integration tests pass
  • All 37 browser tests pass
  • ESM output verified to use import statements for xmldom/xpath

edgarsj added 2 commits March 17, 2026 18:20
…mers

Replace inline require() calls with top-level imports for @xmldom/xmldom
and xpath, and mark them as external in the Rollup module build config.
This ensures ESM output uses proper import statements that work in
Node.js ESM consumers (e.g. "type": "module" projects).

The UMD build continues to bundle these deps inline for browser use.
Browser tests stub out these Node-only deps since the browser path
uses native DOMParser/XMLSerializer/XPath APIs.
@greptile-apps

greptile-apps Bot commented Mar 17, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes an "XML DOM parser not available" error for ESM consumers by replacing inline require() calls for @xmldom/xmldom and xpath with top-level import statements, and marking both as external in the ESM/CJS Rollup builds. The UMD (browser) build intentionally keeps them bundled inline via the new isExternalModuleUmd function. Browser tests are handled by a new stub-node-deps Web Test Runner plugin that intercepts and stubs the two Node.js-only packages. The @peculiar/asn1-* packages are also now externalized for ESM/CJS builds, reducing unnecessary bundling.

Key changes:

  • src/utils/xmlParser.ts: require() calls replaced with top-level ESM imports; the module-scoped xpath import is correctly referenced as xpathLib inside the Node.js code paths. Two pre-existing local const xpath string variables in querySelector/querySelectorAll now shadow the new module-level import (minor, but worth renaming for clarity).
  • rollup.config.js & scripts/build-dist.mjs: isExternalModule split into two functions — isExternalModule (ESM/CJS, marks xmldom/xpath/asn1-* as external) and isExternalModuleUmd (UMD, preserves old behaviour of bundling xmldom/xpath inline for browsers).
  • web-test-runner.config.mjs: New stub-node-deps plugin correctly intercepts browser-test resolution of @xmldom/xmldom and xpath and returns a safe stub; since all browser-context functions check window.DOMParser/document.evaluate first, the stub values are never actually invoked.

Confidence Score: 5/5

  • This PR is safe to merge — the fix is targeted, tests pass, and no regressions are introduced.
  • The approach correctly addresses the ESM/CJS bundling issue. Build config changes are symmetric between rollup.config.js and scripts/build-dist.mjs. Browser and Node.js code paths are properly gated. The only issue found is a minor variable-shadowing style nit in xmlParser.ts that does not affect runtime behaviour.
  • No files require special attention beyond the minor variable-shadowing note in src/utils/xmlParser.ts.

Important Files Changed

Filename Overview
src/utils/xmlParser.ts Replaces dynamic require() calls with top-level ESM imports for @xmldom/xmldom and xpath. Logic is sound; browser vs Node.js dispatch is preserved. Minor: two pre-existing local variables named xpath (string) now shadow the new module-level import.
rollup.config.js Correctly splits the old isExternalModule into isExternalModule (ESM/CJS — externalizes xmldom, xpath, @peculiar/asn1-*) and isExternalModuleUmd (UMD/browser — bundles xmldom/xpath inline). Change is intentional and well-structured.
scripts/build-dist.mjs Mirrors the same isExternalModule/isExternalModuleUmd split as rollup.config.js. Both build files are kept in sync correctly.
web-test-runner.config.mjs Adds a stub-node-deps plugin that intercepts @xmldom/xmldom and xpath imports in browser tests and serves a stub module. The stub exports undefined for the Node.js-only APIs (DOMParser, XMLSerializer, select), which is correct because all browser code paths use window.DOMParser/document.evaluate and never reach the xmldom/xpath branches.
package.json Version bumped to 0.4.0-dev.0. @xmldom/xmldom and xpath remain in dependencies (not moved to peerDependencies), which is correct since they are still transitively installed for consumers.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[xmlParser.ts loaded] --> B{Runtime environment?}
    B -->|Browser| C[Use window.DOMParser\nand document.evaluate]
    B -->|Node.js| D[Use xmldom.DOMParser\nand xpath.select]

    subgraph ESM_CJS["ESM / CJS builds"]
        E["@xmldom/xmldom — external"]
        F["xpath — external"]
        G["@peculiar/asn1-* — external"]
    end

    subgraph UMD["UMD build"]
        H["@xmldom/xmldom — bundled inline"]
        I["xpath — bundled inline"]
        J["@peculiar/asn1-* — bundled inline"]
    end

    subgraph BrowserTests["Browser tests stub-node-deps plugin"]
        K["resolveImport intercepts xmldom and xpath"]
        L["serve returns stub with undefined exports"]
        K --> L
    end

    D --> ESM_CJS
    D --> UMD
    A --> BrowserTests
Loading

Comments Outside Diff (2)

  1. src/utils/xmlParser.ts, line 325 (link)

    P2 xpath variable shadows module import

    The local variable xpath (a string holding an XPath expression) now shadows the top-level import * as xpath from "xpath". This was benign before the PR, but now that xpath is a module-level import, any future reader of this function could be confused—or worse, accidentally introduce a bug by trying to call xpath.select(...) and getting a string instead of the module.

    The same issue appears in querySelectorAll at line 366.

    Consider renaming the local variable to avoid the shadow:

  2. src/utils/xmlParser.ts, line 366 (link)

    P2 xpath variable shadows module import

    Same shadowing issue as in querySelector above — the local string variable xpath shadows the top-level import * as xpath from "xpath". Renaming it keeps intent clear:

Last reviewed commit: db18bba

@edgarsj edgarsj merged commit 58c019e into main Mar 17, 2026
3 checks 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.

1 participant