mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-25 00:33:49 +00:00
* Studio: load the settings tab panels when they are shown, not at launch * Studio: keep a settings panel that fails to load from taking the app down A panel is fetched the first time it is shown, so it can now fail where it could not before: offline, or a page whose entry bundle predates an in-place rewrite of dist/ and still names chunks that have been replaced. The dialog is mounted at the app root and nothing above it catches, so the throw unmounted the whole of Studio rather than one panel. Blocking a panel's module in a browser reproduced it: the dialog, its nav and the rest of the page went. The panel area now sits in an error boundary that offers a reload, and the Suspense fallback is a delayed loading line rather than an empty pane, so a slow first open shows something and a prompt one still shows no flash. Reload rather than retry: React caches a lazy rejection for the life of the page and the browser's module map caches the failed import, so re-importing the same URL rethrows without a new request. index.html is served no-store, so a reload does pick up the current chunk names. tests/settings-tab-panel-loading.test.ts gains a case that walks the JSX and asserts every panel Suspense is inside a class that defines getDerivedStateFromError. tests/studio/playwright_settings_tabs.py drives the real dialog in a browser: all twelve tabs, deep-open, the search jump, and the blocked-module case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: consume a failed settings panel prefetch The idle prefetch warms every panel once the dialog opens, so a chunk it cannot fetch reached the page as an unhandled rejection for a tab nobody had asked for. Reproduced by blocking one panel's module in a browser: the rejection landed on window even though the boundary handled the panel that was actually on screen. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: typecheck the settings smoke entry, and stop the harness settling on a placeholder tsconfig.app.json lists the three existing smoke entries explicitly, so the new one was outside the project and npm run typecheck skipped it. Added, and the harness handle it installs on window is now optional, since app code sits in the same project and must not be able to reach a handle only the harness page installs. tsc --listFiles now names the file. The Playwright harness settled on whatever held still for 600ms. The panel renders from a deferred value, so a switch keeps the outgoing content up until the incoming panel is ready, and on a loaded machine that hand-off outlives the window: one run read the placeholder as the final panel and called a correct recovery a failure. It now refuses to settle on something with almost no content. A run that dies on a cold dev server also writes its report instead of leaving none. * Tighten the settings lazy-panel comments * Name the encoding when the settings harness writes its report tests/test_source_read_encoding.py holds every checked-in file read and write in the test trees to an explicit utf-8, so it does not depend on the platform default and break on Windows the day the file gains a non-ASCII byte. The report write was the one that did not. * Let the select's keyboard scroll settle before the font-scale wheel check Pre-existing flake in this step, not something this branch introduced. The step reads scrollTop straight after keyboard.press, but Radix scrolls the highlighted item into view off the back of that keypress, so the value is a mid-scroll sample: instrumented on the ubuntu CI image the viewport went on to settle 24-35px further down in 20 runs out of 20, on this branch and on its merge base alike. Two things break as a result. The stale sample is not the floor the wheel has to beat, which is why the failure reads '20 -> 44' as though the viewport had moved the wrong way when 44 is simply where the keyboard scroll ended up. And a wheel dispatched into a scroll Chromium is still animating can be swallowed outright, which is the actual failure: at a maximum scrollTop of 243 a working -400 wheel lands on 0 every time. So wait for the scroll to stop before taking the floor, keep the pointer inside a viewport that is not always 40px tall, and re-send the wheel on a bounded retry. A viewport that genuinely refuses the wheel still never moves and still fails, just after more tries. * Clear an unconsumed archive deep-open when settings navigates away * Run the settings tab-panel browser smoke in frontend CI * Keep an archive deep-open when the navigation lands back on Data * Keep a settings scroll target when its own tab is reselected * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Load the crypto polyfill on the settings smoke page * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Trim comments in the settings lazy-loading changes * Hold only the Data panel's own module in the settings deep-open test The abandoned-deep-open step routed every request through a handler that sleeps 2.5s, and that sleep runs on the driver thread, so the whole page's module load queued behind it and arrived at the main thread in one go. On a two-core runner sharing the box with a live Studio that pushed the reopen past its 15s timeout, which reads as a settings dialog that would not open when nothing was wrong with it. Route the Data module alone. The assertion is unchanged and still goes red on the pre-change store: the next ordinary visit to Data reopens the archive listing. * Name the cause when the settings smoke page has navigated away Vite dev proxies /api to 127.0.0.1:8888. With a Studio listening there and no token those calls answer 401, the app's auth handling navigates, and the harness window goes with it, after which every step times out waiting for a dialog that cannot exist. It happens on main too, where the harness is gone before the first open, so it says nothing about the panels. Report it. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothshared@gmail.com> Co-authored-by: oobabooga <112222186+oobabooga@users.noreply.github.com>
176 lines
6 KiB
TypeScript
176 lines
6 KiB
TypeScript
// SPDX-License-Identifier: AGPL-3.0-only
|
|
// Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
|
|
|
|
/**
|
|
* The dialog is closed for the whole launch, yet its twelve tab panels were static imports
|
|
* and so ran before first paint. One static `./tabs/...` edge from anywhere reachable at
|
|
* startup puts them all back, so these assert the import graph, not rendered output.
|
|
*/
|
|
|
|
import assert from "node:assert/strict";
|
|
import { readdir, readFile } from "node:fs/promises";
|
|
import path from "node:path";
|
|
import test from "node:test";
|
|
import { fileURLToPath } from "node:url";
|
|
|
|
import ts from "typescript";
|
|
|
|
const SRC = fileURLToPath(new URL("../src", import.meta.url));
|
|
const SETTINGS = path.join(SRC, "features/settings");
|
|
const DIALOG = path.join(SETTINGS, "settings-dialog.tsx");
|
|
const TABS_DIR = path.join(SETTINGS, "tabs");
|
|
|
|
async function* walk(dir: string): AsyncGenerator<string> {
|
|
for (const entry of await readdir(dir, { withFileTypes: true })) {
|
|
const full = path.join(dir, entry.name);
|
|
if (entry.isDirectory()) yield* walk(full);
|
|
else if (/\.tsx?$/.test(entry.name)) yield full;
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Module specifiers of `import`/`export ... from` declarations, parsed rather than grepped:
|
|
* a deferred `import(...)` is a call expression, so it is never collected.
|
|
*/
|
|
const staticSpecifiers = (file: string, text: string): string[] => {
|
|
const parsed = ts.createSourceFile(
|
|
file,
|
|
text,
|
|
ts.ScriptTarget.ESNext,
|
|
false,
|
|
file.endsWith(".tsx") ? ts.ScriptKind.TSX : ts.ScriptKind.TS,
|
|
);
|
|
const specifiers: string[] = [];
|
|
const visit = (node: ts.Node): void => {
|
|
if (ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) {
|
|
const specifier = node.moduleSpecifier;
|
|
if (specifier && ts.isStringLiteral(specifier)) {
|
|
specifiers.push(specifier.text);
|
|
}
|
|
}
|
|
ts.forEachChild(node, visit);
|
|
};
|
|
ts.forEachChild(parsed, visit);
|
|
return specifiers;
|
|
};
|
|
|
|
/** A tab panel, however the importer spelled the path. */
|
|
const isTabPanel = (specifier: string): boolean =>
|
|
/(^|\/)tabs\/[\w-]+-tab$/.test(specifier);
|
|
|
|
test("the dialog loads every tab panel on demand", async () => {
|
|
const source = await readFile(DIALOG, "utf8");
|
|
|
|
const statics = staticSpecifiers(DIALOG, source).filter(isTabPanel);
|
|
assert.deepEqual(statics, [], `settings-dialog still statically imports: ${statics}`);
|
|
|
|
// One loader per panel on disk, so a tab added later cannot go missing from the map.
|
|
const panels = (await readdir(TABS_DIR)).filter((f) => /-tab\.tsx$/.test(f));
|
|
assert.ok(panels.length >= 12, `only found ${panels.length} tab panels`);
|
|
for (const file of panels) {
|
|
const specifier = `./tabs/${file.replace(/\.tsx$/, "")}`;
|
|
assert.ok(
|
|
source.includes(`import("${specifier}")`),
|
|
`no deferred import for ${specifier}`,
|
|
);
|
|
}
|
|
});
|
|
|
|
test("nothing else in src statically imports a tab panel", async () => {
|
|
const offenders: string[] = [];
|
|
for await (const file of walk(SRC)) {
|
|
if (file.startsWith(TABS_DIR)) {
|
|
// A panel importing a sibling is its own business; it is already lazy.
|
|
continue;
|
|
}
|
|
const text = await readFile(file, "utf8");
|
|
for (const specifier of staticSpecifiers(file, text)) {
|
|
if (isTabPanel(specifier)) {
|
|
offenders.push(`${path.relative(SRC, file)}: ${specifier}`);
|
|
}
|
|
}
|
|
}
|
|
assert.deepEqual(
|
|
offenders,
|
|
[],
|
|
`settings tab panels are back on the startup path via:\n${offenders.join("\n")}`,
|
|
);
|
|
});
|
|
|
|
test("a panel that fails to load cannot take the app down with it", async () => {
|
|
// Nothing above the root-mounted dialog catches, so an uncaught render throw unmounts
|
|
// the whole tree, not one panel.
|
|
const source = await readFile(DIALOG, "utf8");
|
|
const parsed = ts.createSourceFile(
|
|
DIALOG,
|
|
source,
|
|
ts.ScriptTarget.ESNext,
|
|
// Parent pointers: the assertion is about which element encloses which.
|
|
true,
|
|
ts.ScriptKind.TSX,
|
|
);
|
|
|
|
const boundaries = new Set<string>();
|
|
const collect = (node: ts.Node): void => {
|
|
if (ts.isClassDeclaration(node) && node.name) {
|
|
const catches = node.members.some(
|
|
(member) =>
|
|
(ts.isMethodDeclaration(member) || ts.isPropertyDeclaration(member)) &&
|
|
member.name !== undefined &&
|
|
ts.isIdentifier(member.name) &&
|
|
(member.name.text === "getDerivedStateFromError" ||
|
|
member.name.text === "componentDidCatch"),
|
|
);
|
|
if (catches) boundaries.add(node.name.text);
|
|
}
|
|
ts.forEachChild(node, collect);
|
|
};
|
|
ts.forEachChild(parsed, collect);
|
|
assert.ok(
|
|
boundaries.size > 0,
|
|
"settings-dialog defines no error boundary for the lazy panels",
|
|
);
|
|
|
|
const tagName = (node: ts.Node): string | null => {
|
|
if (ts.isJsxElement(node)) return node.openingElement.tagName.getText(parsed);
|
|
if (ts.isJsxSelfClosingElement(node)) return node.tagName.getText(parsed);
|
|
return null;
|
|
};
|
|
|
|
let guarded = 0;
|
|
let total = 0;
|
|
const check = (node: ts.Node): void => {
|
|
if (tagName(node) === "Suspense") {
|
|
total += 1;
|
|
for (
|
|
let parent: ts.Node | undefined = node.parent;
|
|
parent;
|
|
parent = parent.parent
|
|
) {
|
|
const name = tagName(parent);
|
|
if (name && boundaries.has(name)) {
|
|
guarded += 1;
|
|
break;
|
|
}
|
|
}
|
|
}
|
|
ts.forEachChild(node, check);
|
|
};
|
|
ts.forEachChild(parsed, check);
|
|
assert.ok(total > 0, "settings-dialog has no Suspense boundary around the panels");
|
|
assert.equal(
|
|
guarded,
|
|
total,
|
|
`${total - guarded} of ${total} panel Suspense boundaries are not inside ` +
|
|
`one of ${[...boundaries].join(", ")}`,
|
|
);
|
|
});
|
|
|
|
test("the panels are prefetched once the dialog opens", async () => {
|
|
// Without this the first tab click trades the startup cost for an interaction one.
|
|
const source = await readFile(DIALOG, "utf8");
|
|
assert.match(source, /scheduleIdleTask/);
|
|
assert.match(source, /Object\.values\(TAB_LOADERS\)/);
|
|
// It warms unselected panels, so a failed chunk must not reach the page as a rejection.
|
|
assert.match(source, /load\(\)\.catch\(/);
|
|
});
|