feat(core/core-xml): migrate to #platform/* imports#38209
Merged
deyaaeldeen merged 3 commits intomainfrom Apr 24, 2026
Merged
Conversation
908c484 to
875bbe7
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
875bbe7 to
5a86e4f
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates @azure/core-xml to Warp/TypeScript’s #platform/* conditional import pattern so the package builds the correct XML implementation per target (browser DOM vs Node/React Native fast-xml-parser) and standardizes per-target TypeScript/Vitest configuration under config/ and eng/tsconfigs.
Changes:
- Switch public entrypoint to
export … from "#platform/xml"and add apackage.json#importsmapping for browser/react-native/default platform resolution. - Move to per-target tsconfigs under
sdk/core/core-xml/config/and updatewarp.config.ymlto use them. - Add React Native Vitest config + script and align browser Vitest config to shared
eng/vitestconfigs/*.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/core/core-xml/warp.config.yml | Point Warp targets at per-package tsconfigs under config/ (and drop polyfill suffix usage). |
| sdk/core/core-xml/vitest.react-native.config.ts | Re-export shared React Native Vitest config. |
| sdk/core/core-xml/vitest.browser.config.ts | Re-export shared browser Vitest config. |
| sdk/core/core-xml/tsconfig.json | Replace legacy references with per-target config/ references (src/test/samples/snippets). |
| sdk/core/core-xml/src/xml-browser.mts | Simplify parseXML implementation (async flow) and minor catch cleanup. |
| sdk/core/core-xml/src/index.ts | Route XML APIs through #platform/xml instead of a fixed relative import. |
| sdk/core/core-xml/package.json | Add imports["#platform/xml"] mapping; update scripts to use new config locations and add RN test run. |
| sdk/core/core-xml/eslint.config.mjs | Add local flat-config ESLint setup using config/tsconfig.lint.json. |
| sdk/core/core-xml/config/tsconfig.test.react-native.json | New RN test tsconfig extending shared eng/tsconfigs. |
| sdk/core/core-xml/config/tsconfig.test.browser.json | New browser test tsconfig extending shared eng/tsconfigs. |
| sdk/core/core-xml/config/tsconfig.src.react-native.json | New RN src tsconfig extending shared eng/tsconfigs. |
| sdk/core/core-xml/config/tsconfig.src.browser.json | New browser src tsconfig extending shared eng/tsconfigs. |
| sdk/core/core-xml/tsconfig.test.node.json | Removed legacy test tsconfig (replaced by config/tsconfig.test.node.json). |
| sdk/core/core-xml/tsconfig.test.json | Removed legacy aggregator test tsconfig (replaced by package tsconfig.json references). |
| sdk/core/core-xml/tsconfig.src.json | Removed legacy src tsconfig (replaced by per-target config/ tsconfigs). |
| sdk/core/core-xml/tsconfig.src.build.json | Removed legacy build tsconfig (replaced by per-target config/ tsconfigs). |
| sdk/core/core-xml/tsconfig.src.browser.json | Removed legacy browser src tsconfig (replaced by config/tsconfig.src.browser.json). |
| sdk/core/core-xml/tsconfig.snippets.json | Removed legacy snippets tsconfig (replaced by config/tsconfig.snippets.json). |
| sdk/core/core-xml/tsconfig.samples.json | Removed legacy samples tsconfig (replaced by config/tsconfig.samples.json). |
| sdk/core/core-xml/tsconfig.browser.config.json | Removed legacy browser test config tsconfig (replaced by config/tsconfig.test.browser.json). |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jeremymeng
approved these changes
Apr 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrate
@azure/core-xmlto#platform/*conditional imports.Changes
#platform/xml— routes XML parsing/serialization to the correct implementation per platform:xml.ts): usesfast-xml-parser(pure JS, no DOM dependency)xml-browser.mts): uses native DOM APIs (DOMParser,XMLSerializer)xml.ts): same as Node — RN has no DOM, so it usesfast-xml-parserconfig/warp.config.ymlto use per-package tsconfigs instead of root-level onesvitest.react-native.config.tsandtest:react-nativescripttsconfig.*.json/tsconfig.browser.config.jsonto sharedeng/tsconfigs/base configsReact Native rationale
On
main, the RN build accidentally fell back toxml.tsbecause thepolyfillSuffix: "-react-native"target looked forxml-react-native.mtswhich did not exist. This PR makes the RN→Node mapping explicit and intentional:fast-xml-parseris pure JavaScript and works in all runtimes, while the browser implementation depends on DOM APIs (document,DOMParser,Node,XMLSerializer) that are not available in React Native.