Cache network fetches + fix console duplicates
On a typical page the console showed 25-30 duplicate 'Failed to load resource' errors because every consumer hook fired its own copy of the same network request: - useDictionaries: once per `useCityName`/`useStationDisplayName` call (6-10x per render across StationDisplay, PopularRequestItem, mini-list rows, etc.) — now a module-level WeakMap<ApiClient> single-flight cache returns the same in-flight Promise. - usePopularRequests: same pattern across start-page and search- history dropdowns — cached via the same mechanism. - useAppSettings: 7+ callers — cached. Dropped console error count on /ru-ru/ from 29 to 5 (the remaining 5 are WAF 403 infra issues from the dev:full proxy cookie, not code). Also updates e2e specs: - schedule-details-mini-list-scoped: asserts the new single-card rail behaviour (was still checking for the old 3-row flat list). - smoke /xx/smoke: targets `[data-testid=error-page-404]` instead of `text=404` — the latter matches both the <title> tag (hidden by user-agent CSS) and multiple DOM nodes, tripping strict-mode.
This commit is contained in:
@@ -7,11 +7,26 @@
|
||||
*/
|
||||
|
||||
import { useState, useEffect } from "react";
|
||||
import type { ApiClient } from "@/shared/api/client.js";
|
||||
import { useApiClient } from "@/shared/api/provider.js";
|
||||
import { getPopularRequests } from "../api.js";
|
||||
import type { PopularRequest } from "../types.js";
|
||||
import type { ApiError } from "@/shared/api/errors.js";
|
||||
|
||||
// Module-level single-flight cache: multiple components (start pages,
|
||||
// search history dropdowns, etc.) mount the same hook and used to each
|
||||
// fire their own GET /Requests/1/getpopular. Share the in-flight
|
||||
// Promise by ApiClient instance so the request only happens once.
|
||||
const popularCache = new WeakMap<ApiClient, Promise<PopularRequest[]>>();
|
||||
|
||||
function getPopular(client: ApiClient): Promise<PopularRequest[]> {
|
||||
const existing = popularCache.get(client);
|
||||
if (existing) return existing;
|
||||
const promise = getPopularRequests(client);
|
||||
popularCache.set(client, promise);
|
||||
return promise;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fallback popular requests shown when the API is unavailable.
|
||||
* Matches the Angular mock data so the start page renders identically.
|
||||
@@ -65,7 +80,7 @@ export function usePopularRequests(): UsePopularRequestsResult {
|
||||
setLoading(true);
|
||||
setError(null);
|
||||
|
||||
getPopularRequests(client)
|
||||
getPopular(client)
|
||||
.then((data) => {
|
||||
if (!cancelled) {
|
||||
setRequests(data.length > 0 ? data : FALLBACK_REQUESTS);
|
||||
|
||||
@@ -1,14 +1,39 @@
|
||||
/**
|
||||
* Loads the four dictionary endpoints and returns the transformed, ready-to-use
|
||||
* IDictionaries object. Mounts once per session (the map is client-only and
|
||||
* only mounts once). Not intended for multiple concurrent consumers.
|
||||
* IDictionaries object. Every component that needs a name lookup
|
||||
* (`useCityName`, `useStationDisplayName`, etc.) eventually calls this
|
||||
* hook, so we cache the underlying network request at module scope — a
|
||||
* single GET burst per client, no matter how many consumers subscribe.
|
||||
*
|
||||
* Previously every `useCityName` call fired its own `fetchDictionaries`,
|
||||
* producing 6–10 duplicate requests for the same 4 dictionary endpoints
|
||||
* on a single render (once for each StationDisplay, PopularRequestItem,
|
||||
* mini-list row, etc.). The network cost was an order of magnitude
|
||||
* higher than needed and flooded the console with duplicate 403s when
|
||||
* the WAF was angry.
|
||||
*/
|
||||
|
||||
import { useEffect, useState } from "react";
|
||||
import type { ApiClient } from "@/shared/api/client.js";
|
||||
import { useApiClient } from "@/shared/api/provider.js";
|
||||
import { fetchDictionaries } from "./api.js";
|
||||
import { transformDictionaries } from "./transform.js";
|
||||
import type { IDictionariesState } from "./types.js";
|
||||
import type { IDictionariesState, IRawDictionaries } from "./types.js";
|
||||
|
||||
// Module-level single-flight cache keyed by ApiClient instance. The cache
|
||||
// stores the in-flight Promise, so concurrent callers await the same
|
||||
// resolution. We intentionally do NOT bust the cache on error — a second
|
||||
// call would just re-fail; let the caller retry by remounting with a
|
||||
// fresh ApiClient if it wants a retry.
|
||||
const rawCache = new WeakMap<ApiClient, Promise<IRawDictionaries>>();
|
||||
|
||||
function getRawDictionaries(client: ApiClient): Promise<IRawDictionaries> {
|
||||
const existing = rawCache.get(client);
|
||||
if (existing) return existing;
|
||||
const promise = fetchDictionaries(client);
|
||||
rawCache.set(client, promise);
|
||||
return promise;
|
||||
}
|
||||
|
||||
export function useDictionaries(lang: string): IDictionariesState {
|
||||
const client = useApiClient();
|
||||
@@ -21,7 +46,7 @@ export function useDictionaries(lang: string): IDictionariesState {
|
||||
useEffect(() => {
|
||||
let cancelled = false;
|
||||
|
||||
fetchDictionaries(client)
|
||||
getRawDictionaries(client)
|
||||
.then((raw) => {
|
||||
if (cancelled) return;
|
||||
const dictionaries = transformDictionaries(raw, lang);
|
||||
|
||||
@@ -1,6 +1,19 @@
|
||||
import { useEffect, useState } from "react";
|
||||
import type { ApiClient } from "@/shared/api/client.js";
|
||||
import { useApiClient } from "@/shared/api/provider.js";
|
||||
import { getAppSettings } from "@/shared/api/appSettings.js";
|
||||
import { getAppSettings, type AppSettingsResponse } from "@/shared/api/appSettings.js";
|
||||
|
||||
// Module-level single-flight cache so 7+ consumer components don't
|
||||
// each GET /appSettings independently on the same page.
|
||||
const settingsCache = new WeakMap<ApiClient, Promise<AppSettingsResponse>>();
|
||||
|
||||
function getSettings(client: ApiClient): Promise<AppSettingsResponse> {
|
||||
const existing = settingsCache.get(client);
|
||||
if (existing) return existing;
|
||||
const promise = getAppSettings(client);
|
||||
settingsCache.set(client, promise);
|
||||
return promise;
|
||||
}
|
||||
|
||||
const DAYS_PATTERN = /^(\d+)d$/;
|
||||
const HOURS_PATTERN = /^(\d+)h$/;
|
||||
@@ -81,7 +94,7 @@ export function useAppSettings(): UseAppSettingsResult {
|
||||
useEffect(() => {
|
||||
let cancelled = false;
|
||||
|
||||
getAppSettings(client)
|
||||
getSettings(client)
|
||||
.then((response) => {
|
||||
if (cancelled) return;
|
||||
const ob = response.uiOptions?.filter?.onlineboard;
|
||||
|
||||
@@ -1,52 +1,39 @@
|
||||
import { test, expect } from "@playwright/test";
|
||||
|
||||
// On the schedule details page, the left mini-list must:
|
||||
// 1. Show only the CURRENTLY-OPEN flight's instance on each day in the
|
||||
// [-1, +1] window — matching Angular's
|
||||
// `CurrentScheduleService.getScheduleType` / `compareFlightsByPId`
|
||||
// filtering. The old behaviour dumped the entire MOW→MMK route
|
||||
// search into the rail.
|
||||
// 2. Render the open day's row with the SAME combined origin/
|
||||
// destination as its day-±1 siblings — for connecting itineraries
|
||||
// that means Moscow→Murmansk on every row, not first-leg-only
|
||||
// Moscow→St Petersburg on the highlighted row. Earlier we passed
|
||||
// `flights[0]` (the first leg) as `currentFlight`, which produced
|
||||
// a stub row that visually disagreed with the rest of the rail.
|
||||
// On the schedule details page the left mini-list renders a SINGLE
|
||||
// card for the currently-open flight — matching Angular's
|
||||
// `schedule-flights-mini-list`, which only falls into the
|
||||
// multi-day-accordion branch when `schedule.length > 1`. Previously
|
||||
// the rail showed day-±1 siblings that read as visual duplicates of
|
||||
// the open flight, and before that it dumped the whole route search.
|
||||
//
|
||||
// Reference URL: connecting itinerary SU 6188 + SU 6341 (Moscow → St
|
||||
// Petersburg → Murmansk) on 2026-04-26.
|
||||
// For a connecting itinerary the single card must surface BOTH
|
||||
// flight numbers ("SU 6188, SU 6341") and the combined
|
||||
// Moscow→Murmansk origin/destination, not just the first leg.
|
||||
|
||||
const URL =
|
||||
"/ru-ru/schedule/VKO/SU6188-20260426/LED/SU6341-20260427/MMK?request=schedule-route-MOW-MMK-20260427-20260503";
|
||||
|
||||
test("mini-list — flat list scoped to the open SU 6188 itinerary", async ({ page }) => {
|
||||
test("mini-list — one combined card for the open SU 6188+SU 6341 itinerary", async ({ page }) => {
|
||||
await page.goto(URL);
|
||||
|
||||
const miniList = page.locator(".schedule-mini-list");
|
||||
await expect(miniList).toBeVisible({ timeout: 15000 });
|
||||
|
||||
// Day-grouping accordions were removed — rows are flat.
|
||||
// No day-grouping accordion headers, no day-siblings — just one row.
|
||||
await expect(miniList.locator("[data-testid^='mini-list-day-header-']")).toHaveCount(0);
|
||||
|
||||
const items = miniList.locator("[data-testid^='mini-list-item-']");
|
||||
await expect(items.first()).toBeVisible({ timeout: 10000 });
|
||||
// [-1, 0, +1] window for a daily itinerary: 3 rows, all SU 6188.
|
||||
await expect(items).toHaveCount(3);
|
||||
await expect(items).toHaveCount(1);
|
||||
|
||||
// Every row must reference SU 6188 and must NOT contain unrelated
|
||||
// MOW-MMK route-mates (SU 6190 / SU 6699 used to leak in when the
|
||||
// rail was unfiltered).
|
||||
const railText = (await miniList.innerText()).replace(/\s+/g, " ");
|
||||
// Both flight numbers present (Angular surfaces the whole chain).
|
||||
expect(railText).toContain("SU 6188");
|
||||
expect(railText).toContain("SU 6341");
|
||||
// Combined Moscow → Murmansk endpoints — NOT first-leg-only
|
||||
// Moscow → St Petersburg.
|
||||
expect(railText).toContain("Мурманск");
|
||||
expect(railText).not.toContain("Санкт-Петербург");
|
||||
// No unrelated route-mates from the parent search.
|
||||
expect(railText).not.toMatch(/SU\s*6190/);
|
||||
expect(railText).not.toMatch(/SU\s*6699/);
|
||||
|
||||
// The current row (highlighted) must show the same combined
|
||||
// destination as its siblings — Murmansk, not St Petersburg
|
||||
// (otherwise the highlighted row collapses to the first leg only).
|
||||
const cityCount = (railText.match(/Мурманск/g) ?? []).length;
|
||||
expect(cityCount).toBeGreaterThanOrEqual(3);
|
||||
// No row should show St Petersburg as an arrival (that would mean
|
||||
// the current row regressed to first-leg-only).
|
||||
expect(railText).not.toContain("Санкт-Петербург");
|
||||
});
|
||||
|
||||
@@ -54,9 +54,13 @@ test.describe("Smoke tests", () => {
|
||||
await page.goto("/xx/smoke");
|
||||
await page.waitForLoadState("domcontentloaded");
|
||||
|
||||
// The lang layout renders "404 — Unknown locale: xx" for unsupported locales
|
||||
await expect(
|
||||
page.locator("text=Unknown locale").or(page.locator("text=404")),
|
||||
).toBeVisible({ timeout: 10000 });
|
||||
// The lang layout renders a 404 page. Target the visible page-body
|
||||
// copy (the `.error-page__code` "404" badge or the Russian/English
|
||||
// description) rather than `text=404` alone — the latter matches
|
||||
// the <title> tag which is `hidden` by user-agent CSS, and matches
|
||||
// multiple unrelated DOM nodes, tripping strict-mode.
|
||||
await expect(page.getByTestId("error-page-404")).toBeVisible({
|
||||
timeout: 10000,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user