fix(web): премахване на hydration несъответствието на всяка страница (#274) - #300
fix(web): премахване на hydration несъответствието на всяка страница (#274)#300DiyanaDimitrova wants to merge 5 commits into
Conversation
…inks (midt-bg#274) React Router seeds its FrameworkContext nonce from `<ServerRouter nonce>` (needed for the streaming `<script>` chunks), and `<Links>` stamps that nonce on every `<link>` it renders. The client's `<HydratedRouter>` never receives the nonce — it isn't serialized into the hydration handoff — so on the client `<Links>` renders the same links WITHOUT a nonce. The result is a server `nonce="…"` vs client `nonce={undefined}` attribute mismatch on every page: React warns and leaves that subtree in its server state. Pass an explicit empty nonce to `<Links>` so server and client both render `nonce=""` and agree. The stylesheet/icon links never needed a real nonce (CSP is `style-src 'self' 'unsafe-inline'`); scripts keep their real nonce via `<Scripts>` / renderToReadableStream, so the CSP script gate is unchanged. Verified with headless Chrome (CDP console capture): the hydration warning is gone on /, /contracts, /companies, /authorities and /methodology; scripts still carry the per-request nonce.
…bg#274) After the stylesheet-nonce fix, the remaining per-page hydration warning comes from browser extensions (ColorZilla's `cz-shortcut-listen`, Grammarly's `data-gr-*`, password managers, …) that inject attributes onto <body> before React hydrates — React then reports a server≠client attribute mismatch. The app sets no attributes on <body>, so the source is purely the visitor's extensions. Add `suppressHydrationWarning` to <body> — React's documented escape hatch for third-party DOM mutations. It's shallow: it silences mismatches on <body>'s own attributes/text only, never its children, so a genuine app-level mismatch inside the page still surfaces. This clears the last of the console noise midt-bg#274 set out to remove so real mismatches aren't masked.
The review's one actionable note was a missing regression test. A faithful SSR test would need React Router internals (a manifest + data-router state) to reproduce the server/client FrameworkContext nonce split, which is brittle and couples to RR internals. Instead, add a focused AST guard over the exported `Layout`: assert `<Links>` carries an explicit empty `nonce` and `<body>` carries `suppressHydrationWarning`. It fails if either prop is reverted (verified), catching the exact regression without SSR scaffolding. Reads root.tsx via Vite's `?raw` import (typed by vite/client) rather than node:fs, since apps/web test files typecheck under the Workers config which has no Node types.
DiyanaDimitrova
changed the title
Review — verdict: APPROVE (9.5/10)Security-critical scan: CLEANNo secrets, no new URLs (only pre-existing same-origin asset paths), no dependency changes, no obfuscation. The one security-adjacent change — RR-rendered
Correctness — verified against sources, not just the PR description
The regression guard does fail if either prop is reverted ( Non-blocking nits
SummaryAtomic, correctly diagnosed, minimally scoped, comment density matches the repo's house style, and the trade-off (a structural guard instead of a brittle React Router-internals SSR test) is stated honestly rather than papered over. The nits above are polish, not merge blockers. |
nikimilenkov
left a comment
nikimilenkov
left a comment
There was a problem hiding this comment.
Обстоен преглед — PR #300 @ ebfd417 (затваря #274)
Благодаря за този PR — диагнозата е точна, механизмът е проверен от мен до ниво изходен код на React/React Router, и поправката е правилната сред наличните варианти. Прегледът мина по строгия протокол (пет паралелни измерения, всяка находка проследена и възпроизведена върху чисто копие на този HEAD); резултатът е одобрение — бележките по-долу са за прецизност на коментарите и за дълготрайност на workaround-а, не за коректност.
Предложение: одобрение — 0 критични, 0 високи, 3 средни (само документация/поддръжка), 5 ниски. (Съветодателно — решението е на поддържащите.)
Прегледът се разминава с този на @ydimitrof само в детайли и стига до същия извод — присъединявам се към одобрението. Две дребни уточнения към неговите формулировки, установени при проверката на източниците: (1) „stylesheet/icon links surfaced the mismatch“ — предупреждаваха само stylesheet линковете; иконите са React 19 hoistables и се хидратират чрез съпоставяне по href/rel, при което nonce изобщо не се сравнява (потвърдено и от наблюдаваната на живо конзола — икони не фигурират); (2) „criticalCss is a dev-mode Vite feature“ — unstable_getCriticalCss е build hook, не само dev, така че съображението за свързаността със style-src важи и в продукционна посока (вж. СРЕДНА 3).
Изпълнени проверки: @sigma/web 484/484 (потвърдено тук, 44 файла); мутации върху двата prop-а — маха се nonce="" → тестът пада ✓, nonce={undefined} → пада ✓, маха се suppressHydrationWarning → пада ✓; CI зелен (check/semgrep/test/cacbg); сканът за сигурност на diff-а е чист (без тайни/URL-и/зависимости). Механизмът е потвърден в инсталираните пакети, не по памет: RR 7.18.0 if (nonce == null && contextNonce) — изричното "" не е == null и печели; React 19 сериализира nonce="" буквално (проверено с изпълнен renderToStaticMarkup); клиентският FrameworkContext на HydratedRouter наистина няма ключ nonce. Отделно потвърждение от практиката: същото предупреждение наблюдавах на живо върху сроден deployment — листваше точно и само трите rel="stylesheet" линка.
Какво е проверено и вярно
nonce=""е единствената in-tree стойност, която работи. Алтернативата<Links nonce={nonce} />(контекстът на приложението) би пресъздала несъответствието:useNonce()еundefinedна клиента (нямаentry.client.tsx, няма provider), аundefinedсъщо задейства fallback-а. Проверено и че двете „очевидни" алтернативи са архитектурно погрешни: сериализиране на nonce през root loader-а counter-ира edge-cache CSP дизайна (workers/app.ts:87-98— замразен кеш не бива да преиграва nonce), а махането наnonceот<ServerRouter>убива streaming скриптовете под строгия CSP.- CSP повърхността е чиста. Изброено е всичко, което
<Links>рендира в това приложение (2 stylesheet + 4 icon линка; без page-prefetch, без criticalCss — проверено в конфигурацията): stylesheet-ите минават през'self', иконите презimg-src 'self';style-src 'self' 'unsafe-inline'(security.ts:13) няма nonce източник, така че нищо не се неутрализира. Нищо не чете nonce от DOM (grep: 0). Реалният nonce остава само на скриптовете, които го изискват — маргинално по-малко nonce ехо в маркъпа. - „Скриптовете не предупреждават" е вярно, с доказан механизъм: RR слага
suppressHydrationWarning: trueна всеки елемент, който<Scripts>/<ScrollRestoration>издават (вкл. modulepreload линковете) — структурно невъзможно е да предупредят, затова е правилно, че PR-ът не ги пипа. suppressHydrationWarningе плитък — потвърдено в react-dom: гейтва само собствените атрибути/директен текст на елемента, никога наследниците. Приложението не поставя атрибути на<body>преди хидратацията (двете места, които го пипат — a11y widget-ът сclassList.toggleиSiteHeaderсoverflow— са вuseEffect, т.е. след нея).- Тестът убива и трите реални регресии (мутационно проверено);
?rawимпортът и мотивът за него (Workers tsconfig без Node типове) са точни;typescriptе съществуваща devDependency и не може да стигне до клиентския bundle (нула импортьори извън vitest include-а); добавената цена е ~200 ms в един test worker. Никаква runtime цена.
Средни бележки (документация/поддръжка — нищо блокиращо)
- Прецизност на двата коментара в
root.tsx— три твърдения са по-широки от фактите, а бъдещ читател ще действа по тях.
(а) „стampва го на всеки<link>… mismatch на ВСЯКА страница" — предупреждават самоrel="stylesheet"линковете: иконите са React 19 hoistables, които се хидратират чрез съпоставяне поhref/rel(nonce никога не се сравнява там). Съвпада и с наблюдаваното на живо — в конзолата фигурират точно stylesheet линковете.
(б) „приложението не задава никакви атрибути на<body>" — буквално невярно:accessibility.js:106,116(classList.toggle) иSiteHeader.tsx:60го пипат; вярното и важното е „преди хидратацията" — и то държи само защото widget-ът се инициализира отuseEffect. Заслужава си коментарът да каже точно това, защото при промяна на тайминга (напр. изнасяне наgetSettings()в pre-hydration inline скрипт срещу FOUC) подтискането ще замаскира точно този клас разминаване.
(в) Никъде не е казано, че това е само-development проблем: продукционният react-dom изобщо не съдържа диффинг на атрибути при хидратация (проверено — 0 срещания). Изречение „коригираме dev-конзолата, не продукционен дефект" би спестило погрешния извод.
(Дребно: „RR falls back only when the prop is null" — реалната проверка е== null, т.е. иundefined; формулировката леко подвежда.) - Произходът на бъга не е записан, значи workaround-ът няма условие за премахване. Това е регресия от react-router 7.18.0 — CHANGELOG, #15170 „Use the
ServerRouternonce for nonce-aware SSR components…": преди този bump<Links>не четеше контекстния nonce и несъответствието не е съществувало. Едно изречение в коментара (версия + upstream PR) прави workaround-а премахваем при следващ bump. Отделно —<Links>липсва от собствения RR документdocs/how-to/security.md, а fallback-ът е приложен върху клас елементи, които React хидратира чрез диффинг: легитимен upstream доклад. - Инертността на
nonce=""виси на две неизпълними-от-теста условия, невързани към нищо. (а)style-srcда пази'self'— пазено отsecurity.test.ts:11, но без обратна препратка отroot.tsx; (б) празният nonce тече през цялото поддърво на<Links>— criticalCss<style>иPrefetchPageLinksmodulepreload-ите — днес мъртви пътища (проверено: нито page дескриптори, нито criticalCss конфигурация), но при бъдещо включване биха се рендирали с празен nonce тихо. Едно изречение за обхвата в коментара + четириредовият тест, който заковава „<body>няма други атрибути освен подтискането" (AST помощниците в новия файл го правят тривиален), затварят и двете посоки.
Ниски бележки
- Тест-ергономика: (а) семантично еквивалентното
suppressHydrationWarning={true}проваля теста (свръх-строг към формата); (б) преместване на<Links nonce="" />в дъщерен компонент пада сTypeErrorвместо със смислено съобщение — non-null дереференциите на ред 43-44/57/66 се изпълняват предиexpect(layout).toBeDefined()на 59/71, така че „приятелските" assertions са мъртви; (в) тестът пази синтаксис, не поведение — RR bump, който промени fallback семантиката (реалният регресионен вектор), минава през него. Repo-то вече има jsdom +createRoutesStubобразец (conflicts.render.test.tsx), с който може да се твърдиlink.getAttribute('nonce') === ''върху рендиран изход — по-издръжливо на рефакторинг, макар че и то не възпроизвежда истинския SSR/client split (както коментарът в теста честно признава). <html lang="bg">остава извън обхвата — същите разширения инжектират и върху документния елемент (а и a11y widget-ът пишеhtml.classList, макар след хидратацията). В наблюдаваното предупреждение html не фигурира, така че това е въпрос за изрично решение („в обхвата на #274 ли е чистата конзола за посетители с такива разширения?"), не дефект.- Двойният nonce канал вече е излишен: от 7.18.0
<Scripts>/<ScrollRestoration>сами падат към контекстния nonce (проверено на ред 10040/10748 в dist-а), така че подаването презuseNonce()дублира; с новото литерално""вroot.tsxсъжителстват три различни nonce идиома в четири съседни реда. Кандидат за отделен, малък консолидиращ PR — не за този. - Едноредова обратна препратка в
security.ts:5-13(„<Links nonce="">в root.tsx разчита'self'да оторизира stylesheet-ите") би затворила връзката и от другата страна. - Извън обхвата на diff-а, само за отбелязване:
hardenResponse(workers/app.ts:89) буферира целия HTML сawait response.text()по cache-MISS пътя — предсъществуващо, но е гръбнакът, на който стъпва целият CSP аргумент тук.
Какво беше проверено (чеклист)
- Скан за сигурност: чисто (без тайни/URL-и/зависимости; тестът е чист AST parse без изпълнение на код)
- 484/484 потвърдено тук; трите реални мутации падат, двете false-fail форми документирани
- Fallback семантиката, сериализацията на
nonce="", липсата на client nonce и подтискането при<Scripts>— проверени в инсталираните react-dom/react-router източници, не по документация - CSP повърхността изброена елемент по елемент срещу реалната политика; edge-cache CSP пътят недокоснат
- Продукционният bundle няма атрибутен хидратационен диффинг (grep: 0) — dev-only характер потвърден
- Твърденията от описанието на PR-а сверени: CSP низът точен, 484/484 точно, „без body атрибути" — вярно по същество за прозореца на хидратация (двете post-hydration изключения намерени и отчетени)
- Регресионният произход установен: RR 7.18.0 / #15170 (CHANGELOG)
Здравна оценка: 9 / 11
Коректност ✓ · Сигурност ✓ · Производителност ✓ · Тестове ◐ (убива реалните регресии; false-fail форми и синтактичен, не поведенчески пазач) · Коментари/документация ◐ (три твърдения по-широки от фактите; липсващ произход) · Обхват ✓ · Идемпотентност ✓ · Наблюдаемост ✓ · Стил ✓ · Поддръжка ◐ (незаписани условия за премахване/съвместимост) · Честност на валидацията ✓.
Преди сливане (по преценка — нищо не блокира)
- Прецизиране на двата коментара (СРЕДНА 1) и ред за произхода RR 7.18.0/#15170 (СРЕДНА 2) — чисто текстови промени.
- Четириредовият тест за „
<body>без други атрибути" + изречение за обхвата на<Links nonce="">(СРЕДНА 3). - Ниските — по желание; 3 (консолидацията на nonce каналите) е за отделен PR.
Хубава, дисциплинирана поправка: правилният лост, на правилното място, с честно описани ограничения на теста — и с доказуем произход в upstream, който си заслужава да бъде докладван там. Благодаря!
…scope Address the non-blocking review notes on PR midt-bg#300: - root.tsx <Links> comment: only the stylesheet links surface the mismatch (icon links are React 19 hoistables matched by href/rel, where nonce isn't compared); it's a dev-console warning, not a production defect (prod react-dom does no attribute hydration diffing); the RR fallback is `== null` (null/undefined); record the origin (react-router 7.18.0, remix-run/react-router#15170) so the workaround is removable at a future bump; note the empty nonce flows through the whole <Links> subtree (criticalCss <style> / PrefetchPageLinks modulepreloads). - root.tsx <body> comment: correct "no body attributes" -> "no body attributes before hydration" (accessibility.js + SiteHeader mutate it from useEffect), and note the flag would mask a pre-hydration body mismatch. - security.ts: back-reference that <Links nonce=""> relies on style-src keeping 'self' with no nonce source. - root.render.test.ts: add a guard that <body> has exactly one attribute (suppressHydrationWarning); reorder so toBeDefined() runs before the derefs. No runtime change. typecheck + lint + @sigma/web 485/485.
nikimilenkov
left a comment
nikimilenkov
left a comment
There was a problem hiding this comment.
Благодаря за прецизния follow-up, @DiyanaDimitrova — проверих 515dba0 точка по точка срещу прегледа си, със същата дисциплина: пуснах пакета и мутирах новия тест върху този HEAD. Всичко е адресирано; потвърждавам одобрението (съветодателно, решението е на поддържащите).
Проверено на този HEAD
- СРЕДНА 1 (прецизност на коментарите) — и трите твърдения са коригирани точно: „only the stylesheet links surface this mismatch" с назован механизъм (иконите са hoistables, съпоставяни по
href/rel); „the app sets no<body>attributes before hydration" с изричното предупреждение, че pre-hydration inline скрипт би бил маскиран; „development-console warning… production react-dom does no attribute hydration diffing". И формулировката „null/undefined" за fallback-а е точна. - СРЕДНА 2 (произход) — записана: RR 7.18.0 / remix-run/react-router#15170, с условие за премахване („revisit/remove … if a future RR release changes that behaviour"). Точно това прави workaround-а премахваем при следващ bump.
- СРЕДНА 3 (свързаност) — затворена от двете страни: обратната препратка в
security.ts(„<Links nonce="">relies onstyle-srcretaining'self'…") + документираният обхват върху цялото<Links>поддърво (criticalCss / PrefetchPageLinks) + новият трети тест, който заковава<body>към точно един атрибут. Мутирах го за проверка: инжектиранclassNameвърху<body>→ тестът пада с точното съобщение. Това е и предпазителят, който прави подтискането дългосрочно безопасно. - НИСКА (ред на assertions) — поправена:
expect(layout).toBeDefined()и новите пазачи заlinks[0]/bodies[0]вече стоят преди дереференциите, така че липсващ/преименуванLayoutпада със смислено съобщение, не сTypeError. - Пакетът на този HEAD: 485/485 (пуснат тук; новият тест е третият в
root.render.test.ts).
Останалите незадължителни бележки (строгостта към формата {true}, jsdom вариантът на пазача, консолидацията на nonce каналите след 7.18.0) са си по преценка — нито една не е блокираща, а последната така или иначе е за отделен PR.
Чиста, дисциплинирана поправка с ясно записан произход и премахваемост — точно както трябва да изглежда workaround за upstream регресия. Благодаря!
ydimitrof
left a comment
ydimitrof
left a comment
There was a problem hiding this comment.
Обобщение на прегледа
PR-ът адресира конкретен и добре описан проблем: несъответствие при хидратация (hydration mismatch), което води до предупреждения в конзолата на всяка страница. Решението е минимално, целенасочено и изключително добре документирано.
Phase 0 — Критична сигурност: ЧИСТО ✅
- Няма твърдо кодирани тайни (secrets/ключове/пароли).
- Няма нови или променени URL адреси.
- Няма зловредни шаблони (backdoor, инжекция на код, обфускация).
- Няма нови зависимости (
typescript/vitestвече се използват). nonce=""е документиран като инертен при текущияstyle-src 'self' 'unsafe-inline'; коментарът вsecurity.tsизрично обвързва тази инвариантност с промяната. Отлично.
Оценка по измерения
- Архитектура/Коректност (силно): Двете промени (
<Links nonce="" />иsuppressHydrationWarningна<body>) са правилният начин да се изравни server/client markup-ът. Обяснението за поведението на React Router 7.18.0 (FrameworkContext nonce) и за React 19 hoistables е точно и проследимо до upstream issue-то. - Сигурност (агентно ниво): Няма привнесени уязвимости.
suppressHydrationWarningе плитък (shallow) и е ясно документирано, че маскира само собствените атрибути на<body>, не и децата му. - Производителност: Няма регресии; промяната е чисто при рендиране, без runtime разходи.
Резерви (не блокиращи)
- Тестовете са структурни, не поведенчески.
root.render.test.tsпарсваroot.tsxкато суров текст през TypeScript AST и потвърждава, че атрибутите присъстват в изходния код — но не упражнява реалната хидратация, която е коренът на бъга. Авторът честно обяснява защо (нужни са Router internals). Приемливо като компромис, но си струва да се отбележи, че тези тестове няма да уловят бъдеща регресия, при която атрибутите съществуват, но хидратацията пак се разминава. - Крехкост на третия тест — виж inline коментара.
Заключение
Промяната е чиста, атомична и продуктово смислена. Резервите касаят единствено качеството/обхвата на тестовете и не са блокиращи. Препоръка: адресирайте inline бележката преди merge.
| expect(bodies[0]).toBeDefined(); | ||
|
|
||
| const attributes = bodies[0]!.attributes.properties; | ||
| expect(attributes).toHaveLength(1); |
There was a problem hiding this comment.
Тази проверка toHaveLength(1) прави теста крехък: всяко бъдещо легитимно добавяне на атрибут към <body> (напр. className, lang, data-*) ще счупи теста, дори когато suppressHydrationWarning продължава да присъства коректно. Ако целта е да се гарантира именно наличието на suppressHydrationWarning, по-устойчиво е да се провери, че атрибутът присъства (както прави вторият тест), вместо да се фиксира точният брой атрибути. Ако строгостта е умишлена (за да предотврати мълчаливо добавяне на pre-hydration атрибути към <body>), добавете кратък коментар, който да обясни намерението, за да не изглежда като случайно ограничение.
| describe('root Layout hydration guards', () => { | ||
| it('passes an explicit empty nonce to Links', () => { | ||
| expect(layout).toBeDefined(); | ||
| const links = layoutElements('Links'); |
There was a problem hiding this comment.
Тестът валидира само първия <Links> елемент (links[0]). Ако намерението е инвариантата да важи за всеки <Links> в Layout, обмислете проверка върху всички намерени елементи (напр. итерация през links). В текущия код има само един, така че това е само превантивна бележка за устойчивост при бъдещи промени.
3 participants
Затваря #274.
Две независими hydration несъответствия по атрибути се задействаха на всяка страница (шумна конзола, която маскира истински предупреждения). И двете са адресирани в
apps/web/app/root.tsx, плюс регресионен предпазител.1. Nonce върху
<link>за стиловете (грешката в приложението)<ServerRouter nonce>задава nonce-а вFrameworkContextна React Router (нужен за streaming<script>парчетата), а<Links>слага същия този nonce върху всеки<link>, който рендира. Клиентският<HydratedRouter>никога не получава nonce-а (той не се сериализира в hydration handoff-а), затова на клиента<Links>рендира линковете без nonce → на сървъраnonce="…"срещуnonce={undefined}на клиента, на всяка страница.Поправка: подаваме изричен празен nonce на
<Links>— изрична non-null стойност печели пред контекстния nonce (RR пада обратно към контекстната стойност само когато prop-ът е null), така че сървърът и клиентът рендиратnonce=""и съвпадат. Линковете никога не са имали нужда от истински nonce: CSP еstyle-src 'self' 'unsafe-inline'(без nonce за стилове). Скриптовете запазват истинския си per-request nonce през<Scripts nonce>/renderToReadableStream, така че CSP гейтът за скриптове остава непроменен.2. Шум по
<body>от разширенияБраузър разширения (ColorZilla
cz-shortcut-listen, Grammarlydata-gr-*, мениджъри на пароли) инжектират атрибути върху<body>преди React да хидратира. ДобавенsuppressHydrationWarningвърху<body>— документираният escape hatch на React. Той е плитък (заглушава несъответствия само по собствените атрибути/текст на<body>, никога по наследниците), а приложението не задава никакви атрибути на<body>, така че нито едно истинско несъответствие не се маскира.3. Регресионен предпазител
apps/web/app/root.render.test.ts— фокусиран AST предпазител върху експортиранияLayout, който твърди, че<Links>носи изричен празенnonce, а<body>носиsuppressHydrationWarning. Достоверен SSR тест би изисквал крехки вътрешности на React Router; този хваща точната регресия (проверено — пада, ако някой от двата prop-а бъде върнат) без това обвързване.Проверка
/,/contracts,/companies,/authorities,/methodology; изчезва след поправката; SSR емитваnonce=""върху линковете за стилове, докато скриптовете запазват истинския nonce.pnpm typecheck·pnpm lint·@sigma/web484/484.