Repository navigation
fix(jsdom): keep File bytes in Request/Response body and formData on Node 24 - #11300
harshit-d3v wants to merge 1 commit into
Conversation
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
🟡 Changes recommended
Address the global File race and preserve the default Blob filename.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes Node 24 File/FormData interoperability in the jsdom environment while preserving multipart bytes and jsdom-compatible results.
Changes:
- Wraps multipart blobs in Node
Fileobjects. - Patches
Request/Response.formData()conversion. - Adds multipart body and round-trip regression tests.
File summaries
| File | Summary | Findings |
|---|---|---|
test/unit/test/environments/jsdom.spec.ts |
Adds multipart request and response coverage. | None |
packages/vitest/src/integrations/env/jsdom.ts |
Adds Node/jsdom File and FormData compatibility handling. |
Critical (3 votes): concurrent calls can race on the global File. Moderate (1 vote): plain Blob entries should preserve the default "blob" filename. |
Review details
Suppressed comments (1)
packages/vitest/src/integrations/env/jsdom.ts:351
- For a plain
Blob,value.nameis undefined, but theFormData.append(name, blob)contract uses the default filename"blob". Passing that undefined value toFilechanges multipart serialization for Blob entries (and can produce anundefinedfilename); preserve the default withvalue.name ?? 'blob'.
nodeFormData.append(key, new NodeFile_([compatBlob], value.name, { type: value.type, lastModified: value.lastModified }))
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const File = globalThis.File | ||
| globalThis.File = NodeFile_ | ||
| let formData: FormData | ||
| try { | ||
| formData = await original.call(this) |
There was a problem hiding this comment.
good catch. fixed by serializing the calls through one shared lock, so overlapping formData() (across Request and Response too) run one at a time and never see each other's global swap. added a concurrency test that runs two parses in parallel and checks the global is restored.
…Node 24 undici 7 (Node 24.6+) resolves `File` from the global at call time, and the jsdom env swaps it for jsdom's File. So a Blob sent through a Request body serialized as "[object Blob]", and `request.formData()` threw when parsing a multipart body. Wrap blobs in Node's File before undici sees them, and restore Node's File around `formData()`, then hand back jsdom FormData/File so `instanceof` still holds in tests.
7cd8df4 to
7934761
Compare
…keits-Updates (#816) * fix(karte): Klick beim Linienzeichnen nicht mehr von der Beschriftung verschluckt Die laufende Summe am Mauszeiger, die Vorschaulinie und die Kupplungsstriche liegen in derselben Zeichenebene wie die gesetzten Punkte. Der Klickfänger ließ alles aus dieser Ebene durch, und die drei waren trotz `interactive: false` anklickbar — die Angabe stand in `pathOptions`, und von dort wirkt sie nicht: Leaflet liest `interactive` einmalig in `_initPath`, react-leaflet reicht `pathOptions` erst danach über `setStyle` nach, die Klasse `leaflet-interactive` bleibt gesetzt. Damit lag unter dem Zeiger eine unsichtbare Klickfläche, dazu die Vorschaulinie als Trefferband. Am Laptop ließ sich kaum noch ein Punkt setzen; auf Touch-Geräten fiel es nicht auf, weil es dort ohne `mousemove` keine Vorschau gibt. - `interactive` als Eigenschaft statt in `pathOptions`, auch an den Leitungsvorschlägen des Assistenten - der Klickfänger lässt nur noch Treffer auf einem gesetzten Punkt durch * feat(karte): Koordinaten händisch eintragen, in drei Schreibweisen Kommen Koordinaten von außen — von Polizei oder LSZ —, ließ sich damit bisher nichts anfangen: Beim Anlegen eines Elements fehlten die Felder ganz (`showLatLng={!!item.id}`), und in der Seitenleiste zeigte die Karte die Position zwar an, blendete sie beim Bearbeiten aber aus. Jetzt steht die Position in Dezimalgrad, in Grad/Minuten/Sekunden und in Grad/Dezimalminuten, und jedes dieser Felder nimmt ein ganzes Paar in jeder der drei Schreibweisen an — mit oder ohne Gradzeichen, Himmelsrichtung vorn oder hinten, Punkt oder deutsches Komma. Steht die Himmelsrichtung dabei, ist die Reihenfolge von Breite und Länge belanglos. - `src/common/coordinates.ts` liest und schreibt die drei Schreibweisen - `CoordinateFields` hält immer nur einen Entwurf: das Feld, in dem getippt wird. Alle übrigen zeigen die gespeicherte Position, also gibt es keinen Gleichlauf zwischen vier Zuständen zu pflegen - die Felder erscheinen auch am neuen Element und in der Seitenleiste * feat(einsatzmittel): Fahrzeuge fremder Organisationen einfärben Auf einer Lage mit Rettung, Polizei und Nachbarwehren sind zwanzig rote Balken kein Bild der Lage, sondern ein Haufen. Ein Einsatzmittel trägt deshalb zwei neue Felder: den Schalter „Fremdorganisation" und eine freie Farbe. - `vehicleMarkerColor()` nimmt die gewählte Farbe, sonst Blau bei `fremd` und sonst Rot. Der Schalter setzt die Farbe also nicht, er ändert nur die Vorgabe — wer einmal eine eigene gewählt hat, verliert sie beim Umschalten nicht - das Popup weist ein Fremdfahrzeug als solches aus - an Besatzung, ATS-Trägern, Stärketabelle und Personal-Board ändert der Schalter nichts; die Begründung steht in docs/einsatzmittel-staerke.md Dazu `sanitizeHexColor`: Der Farbwähler des Elementdialogs liefert acht Stellen (`format="hex8"`), die bisher als „overly long" abgewiesen wurden — jede dort gewählte Farbe wäre auf den Vorgabewert zurückgefallen. Gültig sind jetzt genau die vier Längen, die CSS kennt (3, 4, 6, 8); die ungeraden Längen dazwischen, die vorher durchgingen, nicht mehr. * feat(einsatzmittel): Fremdkräfte getrennt in der Stärketabelle Fahrzeuge fremder Organisationen zählen nicht mehr stillschweigend zu den eigenen Kräften. Sobald eines als fremd gekennzeichnet ist, teilt sich die Stärketabelle in „Eigene Kräfte" und „Fremdkräfte" mit je einer Zwischensumme und der Gesamtsumme darunter. Ohne Fremdfahrzeuge bleibt die Tabelle unverändert eine Liste mit einer Gesamtzeile. „Wie viele eigene Leute habe ich" und „wer ist sonst noch da" sind zwei Fragen; eine gemeinsame Zahl beantwortet keine von beiden. Die Feuerwehr (fw) taugt dafür nicht als Merkmal — eine Nachbarwehr trägt dort ebenfalls einen Feuerwehrnamen. calculateStrength() liefert dazu neben der Gesamtsumme zwei Gruppen derselben Bauart (eigene, fremde). Der CSV-Export bekommt die Spalte „Kräfte". * fix(print): Einsatzkarte im PDF nicht mehr verzerrt und zerschnitten Die Kartenaufnahme des PDF-Exports bekam eine relative Breite und eine absolute Pixelhöhe vom Bildschirm. html2pdf rendert aber nicht den Bildschirm ab, sondern legt einen Klon des Dokuments in einem Container neu um, der exakt den Satzspiegel breit ist (190mm ≈ 718px). Dabei schrumpft die Prozentbreite, die Pixelhöhe nicht — daher das gekippte Seitenverhältnis und die Höhe, die über die Seitengrenze reicht. Dazu lag die Aufnahme innerhalb der Karten-Flexzeile mit `overflow: hidden`, `height: 100%` und der Seitenleiste als Geschwister. Diese Vorgaben lösen sich im Klon anders auf und beschnitten das Bild, weshalb im PDF oft ein anderer Ausschnitt stand als in der Anzeige. printMapSnapshot.ts rechnet die Pixelmaße der Aufnahme auf den Satzspiegel um und setzt beide Maße absolut in mm; die Höhe ist auf 240mm gedeckelt, weil `pagebreak.avoid` nur für Elemente bis Seitenhöhe greift. Ersetzt wird der ganze Kartenbereich (neue Klasse `.map-area`) statt nur des Leaflet-Elements, damit die Aufnahme aus dem `overflow: hidden` und der Flexzeile herauskommt. Die Karte wird jetzt in `finally` wiederhergestellt: scheiterte die PDF-Erzeugung, blieb die Seite bisher ohne sichtbare Karte stehen. * feat(fahrtenbuch): km-Endstand in der Einsatzzeile statt Zusatzfahrer In der Sammelerfassung zum Einsatz steht in jeder Zeile jetzt der Kilometer-Endstand zur Korrektur — die eine Zahl, die an einer Fahrt wirklich einzutragen ist. Die geschätzte Strecke bleibt Platzhalter, damit sie im Nachweisdokument nicht wie eine Ablesung aussieht. Der Zusatzfahrer ist der Ausnahmefall und zieht in die Details. * fix(print): Print-Seite auf die Satzspiegelbreite von A4 festlegen Beim Drucken über das System zeigte die Karte nur ihre linke obere Ecke. Ursache ist die allgemeine Druckregel in globals.css, die jede Leaflet-Karte auf 100% x 400px umstellt: Leaflet setzt die Kachelebene einmal für die gemessene Containergröße und rechnet sie nie nach. Bricht der Container erst im Druck um, bleibt die Kachelebene stehen und der neue, kleinere Ausschnitt zeigt deren linke obere Ecke. Die Print-Seite ist deshalb jetzt exakt den Satzspiegel breit — 190mm entsprechen 718 CSS-Pixeln, weil das CSS-Pixel im Druck über 1in = 96px festgelegt ist — und zwar auf dem Bildschirm genauso wie auf dem Papier. Damit bricht beim Drucken nichts mehr um und es gibt keinen Versatz. Die Karte bekommt dazu feste Pixelmaße (718x500, entspricht 190x132mm) statt der 80%-Breite aus Map.tsx; die Seitenleiste entfällt auf der Print-Seite, sie ist Bedienoberfläche und nähme der Karte nur Breite. Die allgemeine Druckregel bleibt für die übrigen Seiten bestehen, dort ist sie der einzige Weg zu einer Höhe überhaupt. * feat(karte): Koordinaten erst auf Klick bearbeitbar Vier Zahlenfelder standen an jedem Element offen, auch wenn nur der Name geändert werden sollte — und beim Anlegen fehlten sie ganz, weil `showLatLng={!!item.id}` die Felder an eine schon vergebene ID band. Jetzt steht die Position als Text da, wie sie auch in der Seitenleiste steht, daneben ein Stift. Der klappt die drei Schreibweisen zum Eintragen auf, und der Haken schließt sie wieder. Damit ist die Anzeige der Regelfall und das Eintragen der Handgriff, den man ausdrücklich macht. - Beim Zuklappen zerfällt der Entwurf; halb Getipptes bleibt nicht stehen - Ohne Position steht „keine Position", der Stift ist trotzdem da — genau der Fall, für den das Feld gebaut wurde: eine Koordinate von der Polizei für eine Markierung, die es noch nirgends gibt - `showLatLng={!!item.id}` entfernt, die Felder erscheinen auch am neuen Element * feat(karte): Koordinaten als UTM, Bundesmeldenetz und Kartenlink Drei Schreibweisen waren zu wenig für das, was tatsächlich hereinkommt: Von der ÖK und vom Bundesheer kommt UTM, aus dem Kataster und dem Burgenland-GIS kommt das Bundesmeldenetz, und aus einem Telefonat kommt heute meistens gar keine Zahl, sondern ein geteilter Standort. - `coordinates-grid.ts` rechnet UTM und BMN über proj4, das über das Höhenmodell ohnehin schon im Client-Bundle liegt. Die UTM-Zone folgt aus der Länge — Österreich liegt in zweien, Grenze bei 12° Ost. Beim BMN verrät der Rechtswert den Meridianstreifen, weil die falschen Rechtswerte 150/450/750 km weit genug auseinanderliegen - Gegengeprüft wird die BMN-Rechnung an einem echten Hydranten aus dem Burgenland-GIS, den `hydrantenCsvConverter.test.ts` schon führt — die Zahlen stammen also nicht aus dieser Implementierung - Eine BMN-Angabe muss im beanspruchten Streifen landen. Ohne diese Probe wird aus einem UTM-Paar im falschen Feld klaglos ein Punkt irgendwo auf der Welt - `parseCoordinatePair` liest Kartenlinks: `geo:`, Google, Apple, OpenStreetMap. Ein Kurzlink wird nicht aufgelöst — die Position steht erst hinter der Weiterleitung, und die gibt es im Einsatz womöglich nicht - `EPSG:31257`/`31258` ergänzt, damit auch M28 und M31 gehen - neu: docs/koordinaten.md, samt dem, was bewusst fehlt (MGRS, what3words, Plus Codes) * chore(deps): Abhängigkeiten aktualisieren, jsdom auf 30.0.1 festgenagelt Übernimmt die offenen Dependabot-PRs #812, #813, #814 und #815: - Sammel-PR #812 mit 21 Updates (next 16.3.5, @mui/x-* 9.14.0, next-intl 4.14.6, zod 4.6.5, vitest 5.0.1, moment 2.31.0 u.a.) - @googleapis/drive 25 → 26, gmail 21 → 22, sheets 17 → 18 Die drei @googleapis-Majors brechen nur die Mindest-Node-Version auf 22; gebaut und getestet wird überall mit Node 24. jsdom bleibt bewusst auf 30.0.1, exakt gepinnt und in dependabot.yml ignoriert: ab 30.1 liegt die Blob-Implementierung hinter einem privaten Feld, vitests makeCompatBlob findet sie über getOwnPropertySymbols nicht mehr und URL.createObjectURL stirbt mit „Cannot read properties of undefined (reading '_buffer')". Genau daran scheitern die Tests in Dependabot-PR #812. Der Fix steht upstream aus (vitest-dev/vitest#11300), Begründung in docs/build-und-toolchain.md.
- useNoteSave: opening never writes; the first edit saves once (debounced PATCH with
the version read and an idempotency key); a 409 shows a conflict. T-P1-17-02 green.
- makeTask: a checklist item becomes a task (one POST /v1/tasks) and then a
tumnis://task mention. StarterKit's TrailingNode is off: Markdown is the stored form,
so an empty trailing paragraph has nothing to save. T-P1-17-04 green.
- Editor/MarkdownField behind LazyEditor (React.lazy): Tiptap stays out of the initial
chunk graph. Vitest gets a "build" project (Node environment) for tests/, where the
bundle test builds the app. T-P1-17-18 green.
- PacketPreview (GET /v1/tasks/{id}/packet?kind=enrich) with a "Preview packet" toggle
in the task drawer; it validates only the body it reads. T-P1-17-16 green.
- KnowledgeSection in the Context rail (laptop and phone): count and quota summary,
add text or link, upload, pin with the version read, trash with Undo. Uploads go
through a new lib/fetch apiUpload (the REL-2 rule: writes only through lib/fetch).
Default MSW handlers answer the section's reads. T-P1-17-17 green (both tests).
- Test harness: Node's File/Blob/FormData under jsdom (vitest-dev/vitest#11300).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Hello @harshit-d3v. Thank you for taking the time to contribute! Unfortunately, the team accepts pull requests only from maintainers and approved contributors, so this pull request was closed automatically. We are sorry about that, it is not a judgement of your work. The number of pull requests grew beyond what the team can review, and this policy gives maintainers the space to triage and prioritize issues at their own pace. Please keep the discussion in #9135. Your changes are not lost: a maintainer can reopen this pull request if the team decides to go forward with it. See our pull request policy for more context. |
Description
Follow-up to #11295. Under
environment: 'jsdom'on Node 24.6+, aBlob/Filesent through aRequest/Responsebody serialized as[object Blob], andrequest.formData()/response.formData()never resolved (or threw) when parsing a multipart body.Cause: undici 7 (Node 24.6+, nodejs/undici#4362) resolves
Filefrom the global at call time instead of capturing it at load. The jsdom env replaces the globalFilewith jsdom's, so undici builds each part with jsdom'sFileand then brand-checks it against Node's, which fails.Fix, in
packages/vitest/src/integrations/env/jsdom.ts:makeCompatFormDatawraps each blob in Node'sFilebefore appending, so the serialized body carries the bytes.Request.prototype.formData/Response.prototype.formDatarestore Node'sFileon the global for the duration of the call, then convert the result back to jsdomFormData/Filesoinstanceofstill holds in tests. The patch is removed on teardown.Adds tests that fail without the change on Node 24: a multipart
Requestbody read back with.text(), andrequest.formData()/response.formData()round-trips. Verified on Node 22 and 24.Resolves #9135
Tests
pnpm test:ci.I used Claude to help find and fix this. I reproduced it myself and reviewed the change.