Skip to content
Open
2 changes: 1 addition & 1 deletion scripts/build/deps/webkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* for local mode. Override via `--webkit-version=<hash>` to test a branch.
* From https://github.com/oven-sh/WebKit releases.
*/
export const WEBKIT_VERSION = "549170099226f816a4b204ea1d8fa102fb79eefa";
export const WEBKIT_VERSION = "autobuild-preview-pr-360-52f23e4f";
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/**
* WebKit (JavaScriptCore) — the JS engine.
Expand Down
78 changes: 78 additions & 0 deletions test/js/web/intl/intl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,84 @@
});
});

// ---------------------------------------------------------------------------
// String.prototype.localeCompare — collator caching for (string locale, no options)
// ---------------------------------------------------------------------------

describe("String.prototype.localeCompare with a string locale", () => {
// JSC caches a per-global IntlCollator for a.localeCompare(b, "<locale>") with undefined
// options so that arr.sort((a, b) => a.localeCompare(b, "en")) does not rebuild a UCollator
// on every comparison. These tests guard against the cache returning stale results.

test("matches new Intl.Collator(locale).compare", () => {
for (const loc of ["en", "sv", "de", "ja", "fr", "de-u-co-phonebk"]) {
const cmp = new Intl.Collator(loc).compare;
for (const [a, b] of [["ä", "z"], ["ä", "ae"], ["a", "A"], ["abc", "abd"]] as const) {
expect(a.localeCompare(b, loc)).toBe(cmp(a, b));
}
}
});

test("changing the locale string returns the new locale's order", () => {
// Swedish puts ä after z, English before.
expect("ä".localeCompare("z", "en")).toBeLessThan(0);
expect("ä".localeCompare("z", "sv")).toBeGreaterThan(0);
expect("ä".localeCompare("z", "en")).toBeLessThan(0);
// German phonebook sorts ä with ae.
expect("ä".localeCompare("ae", "de")).toBeLessThan(0);
expect("ä".localeCompare("ae", "de-u-co-phonebk")).toBeGreaterThan(0);
expect("ä".localeCompare("ae", "de")).toBeLessThan(0);
});

test("invalid locale still throws; cache is not poisoned", () => {
expect("ä".localeCompare("z", "en")).toBeLessThan(0);
expect(() => "a".localeCompare("b", "bad locale!!")).toThrow(RangeError);
expect("ä".localeCompare("z", "en")).toBeLessThan(0);
});
Comment thread
robobun marked this conversation as resolved.

test("does not leak into calls that pass options", () => {
expect("a".localeCompare("A", "en", { sensitivity: "base" })).toBe(0);
// The next call must use the default sensitivity, not the {base} collator above.
expect("a".localeCompare("A", "en")).not.toBe(0);
});

Check warning on line 182 in test/js/web/intl/intl.test.ts

View check run for this annotation

Claude / Claude Code Review

'does not leak into calls that pass options' test relies on prior test to warm the cache

The test title says the cache "does not leak *into* calls that pass options" (cached no-options `'en'` collator must not be reused when `{sensitivity:'base'}` is passed), but the body only explicitly asserts the reverse direction. Line 179's `.toBe(0)` does happen to cover the title's direction, but only because the preceding test leaves `'en'` in the per-global cache — run in isolation via `-t`, the cache is cold and that path isn't exercised. Consider adding an explicit warm-up call first so t
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.

test("non-string locales and no-arg form still work", () => {
expect("ä".localeCompare("z", ["sv"])).toBeGreaterThan(0);
expect("ä".localeCompare("z", ["en"])).toBeLessThan(0);
expect(typeof "a".localeCompare("b")).toBe("number");
Comment thread
robobun marked this conversation as resolved.
Outdated
});

test("cached collator survives GC", () => {
"a".localeCompare("b", "fr");
Bun.gc(true);
expect("ä".localeCompare("z", "fr")).toBeLessThan(0);
Bun.gc(true);
expect("ä".localeCompare("z", "fr")).toBeLessThan(0);
});
Comment thread
robobun marked this conversation as resolved.

test("reuses the collator: sort by localeCompare(b,'en') is as fast as a hoisted Intl.Collator", () => {
// A 3000-element sort does ~33k comparisons. With the cache this costs one
// ucol_open instead of ~33k, so the two sorts should take about the same time.
// Without the cache the ratio is >5 on a debug build and >20 on a release build.
const N = 3000;
const arr = Array.from({ length: N }, (_, i) => ((i * 2654435761) >>> 0).toString(36) + "aä"[i % 2]);

const warm = arr.slice(0, 50);
[...warm].sort((a, b) => a.localeCompare(b, "en"));
[...warm].sort(new Intl.Collator("en").compare);

let t = performance.now();
[...arr].sort((a, b) => a.localeCompare(b, "en"));
const withLocale = performance.now() - t;

t = performance.now();
[...arr].sort(new Intl.Collator("en").compare);
const hoisted = performance.now() - t;

expect(withLocale / hoisted).toBeLessThan(3);

Check warning on line 217 in test/js/web/intl/intl.test.ts

View check run for this annotation

Claude / Claude Code Review

Single-shot wall-clock ratio in perf test may flake under CI noise

Consider hardening the perf assertion with a best-of-N loop: it currently takes one `performance.now()` sample per side and asserts `withLocale / hoisted < 3`, so a single GC pause or scheduler hiccup during the first sort (but not the second) could push the ratio past 3 on a debug+ASAN CI runner even when caching works. Running each sort ~3 times and comparing `Math.min(...withLocale) / Math.min(...hoisted)` filters one-off pauses while keeping the >5x/>20x regression signal intact.
Comment thread
robobun marked this conversation as resolved.
Outdated
});
});

// ---------------------------------------------------------------------------
// Segmenter — brkitr/* raw (incl. cjdict)
// ---------------------------------------------------------------------------
Expand Down
Loading