feat(fs-http)!: remove DOM coupling from download/preview (0.3.0) — closes #59#62
Conversation
Deploying fs-packages with
|
| Latest commit: |
a2a167a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0ebafaee.fs-packages.pages.dev |
| Branch Preview URL: | https://feat-fs-http-remove-dom-coup.fs-packages.pages.dev |
|
A few notes from the kendo side after auditing how this lands for us: 1. Call-site count — audit may be offThe PR body says "Audit of production call sites: 5 across kendo/ublgenie/emmie." For kendo alone I count 4:
That leaves only 1 between ublgenie + emmie combined, which feels low. Worth re-running the audit before the post-publish campaign so a consumer file doesn't slip through. 2. Make
|
The merge-base changed after approval.
bd64102 to
be83336
Compare
|
Thanks @jasperboerhof — addressed all three. 1. Audit countYou're right, my count was wrong. Re-ran the audit and the geography is 4 in kendo + 1 in ublgenie, not three territories. Emmie's downloads call its internal 2. Old-call-signature → hard TS errorAlready structurally satisfied. The new signature is
Verified against the rebased branch's 3. Consumer mocks won't surface the regressionCaptured for the post-publish migration brief — will lead with this. Also worth noting: kendo's Branch rebased onto current |
Closes #59. fs-http should be HTTP transport, not browser download UX. The previous downloadRequest/previewRequest methods constructed Blobs, created `<a>` elements, dispatched clicks, and managed object URL lifecycles — coupling that bled into every consumer's tests as fragile global stubs (vi.stubGlobal of `Blob`/`document`/`window.URL`), exposed to vitest 4's class-mock requirement and oxfmt's arrow-function collapse. Three territories independently rediscovered the mitigation. Reshape both methods to pure transport: `(endpoint, options?) → Promise<AxiosResponse<Blob>>`. The DOM-side download dance moves to `@script-development/fs-helpers` ≥ 0.1.2 as `triggerDownload(blob, filename)`. Object-URL lifecycle for previews is now the consumer's concern (one `URL.createObjectURL` line, plus revocation on cleanup). BREAKING CHANGES (fs-http 0.2.0 → 0.3.0): - downloadRequest(endpoint, documentName, type?) → AxiosResponse becomes downloadRequest(endpoint, options?) → AxiosResponse<Blob>. - previewRequest(endpoint) → string (object URL) becomes previewRequest(endpoint, options?) → AxiosResponse<Blob>. - Removed HEADERS_TO_TYPE map (one-entry OOXML lookup, did not earn its place in transport-layer code; consumers can supply their own if needed). Cascade workspace bumps to keep the lock coherent: - fs-helpers 0.1.1 → 0.1.2 (additive: triggerDownload export + happy-dom devDep). - fs-adapter-store 0.1.5 → 0.1.6 (peer range widened to include ^0.3.0 — not a behavior change; doesn't depend on download/preview). - fs-loading 0.1.1 → 0.1.2 (same widening; same rationale). Out of scope (separate dispositions): - streamRequest's `document.cookie` XSRF read remains. It's the only remaining DOM touch in fs-http; cleanup would require either a required `xsrfHeader` option or a cookie-reader middleware shipped alongside, both bigger migrations than #59 covers. - Consumer territory upgrades. Land this PR after #60, then run a post-publish migration campaign across kendo, ublgenie, emmie, entreezuil, and BIO (5 download/preview call sites in production code; ~50 in test mocks). Verified locally: - 19 test files / 430 tests pass (was 18/430; +2 fs-helpers, -2 net fs-http after dropping obsolete DOM-orchestration tests). - 100% coverage maintained on fs-http and fs-helpers. - mutation: fs-http 97.30% (up from 95.74%; less surface to mutate), fs-helpers 100% (including the new dom-download.ts). - All 8 CI gates locally green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
be83336 to
a2a167a
Compare
Summary
Closes #59. fs-http should be HTTP transport, not browser download UX. This PR strips all DOM coupling from
downloadRequest/previewRequestand moves the<a>-click-revoke dance to@script-development/fs-helpersastriggerDownload(blob, filename). fs-http's tests no longer stubBlob,document, orwindow.URL.Why
Per #59, three territories (kendo, ublgenie, emmie) independently discovered that vi.stubGlobal patterns for
Blob/document/window.URLare fragile under formatters and require vitest 4 class-based mocks. The right fix isn't a class-mock recipe in every consumer; it's removing the coupling at the source. fs-http no longer touches the DOM in download/preview paths.Branch base
Rebased onto
mainafter PR #60 (timeout) and PR #61 (content-type narrowing) merged. PR #61'sasStringhelper is dead code under the new API and was dropped in the rebase resolution along with its two defensive tests.Merge order: this PR → consumer migration campaign.
Changes
fs-http (BREAKING — 0.2.0 → 0.3.0)
downloadRequest(endpoint, documentName, type?) → AxiosResponsebecomesdownloadRequest(endpoint, options?) → AxiosResponse<Blob>. No DOM side effects.previewRequest(endpoint) → string(object URL) becomespreviewRequest(endpoint, options?) → AxiosResponse<Blob>. Caller manages URL lifecycle.HEADERS_TO_TYPEmap. The one-entry OOXML lookup did not earn its place in transport code; consumers can supply their own if needed.fs-helpers (additive — 0.1.1 → 0.1.2)
triggerDownload(blob, filename)export — the<a>-click-revoke dance with the same comment about why click() captures its own ref before revoke.Cascade widening (workspace coordination, not behavior change)
fs-adapter-store0.1.5 → 0.1.6: fs-http peer^0.1.0 || ^0.2.0→^0.1.0 || ^0.2.0 || ^0.3.0.fs-loading0.1.1 → 0.1.2: same widening.Both packages keep working unchanged — they don't call download/preview. The peer widening is required so the lock resolves fs-http to the workspace 0.3.0 instead of pulling a registry copy of 0.2.0 into nested
node_modules/.Migration recipe (for consumer territories — separate post-publish campaign)
Audit of production call sites (corrected after @jasperboerhof's review): 5 = 4 in kendo + 1 in ublgenie. Emmie's downloads use its internal
vue-services/httpworkspace package, not@script-development/fs-http, so they are out of scope for this PR's migration. BIO and entreezuil have no production call sites — only mock-server stubs that need a one-line signature update when the version bump lands. Test mocks update to match the new return shape.Out of scope
document.cookieXSRF read. This is the only remaining DOM touch in fs-http after this PR. Cleanup would require either a requiredxsrfHeaderoption or a cookie-reader middleware shipped alongside — both bigger migrations than fs-http: invert Blob/document-global coupling in downloadRequest for testability #59 asks for. File a separate issue if/when prioritized.workspace:*protocol for internal deps. Would eliminate the cascade-widening tax on every fs-http minor bump. Worth a separate ADR.CI gates
All 8 gates green locally:
dom-download.spec, net -2 in fs-http after dropping obsolete DOM-orchestration tests). 100% coverage maintained on fs-http and fs-helpers per their thresholds.dom-download.ts)Test plan
triggerDownloadcovers anchor creation, click, revocation ordernpm run test:coverage430/430vue-services/http, not affected.Refs
feedback_oxfmt_constructor_mocks.md— captures the unsafe-collapse pitfall and class-mock mitigation that motivated the issue.campaigns/ublgenie/2026-04-29-oxfmt-prettier-migration.md— narrative of how this surfaced.