Skip to content

Memoize getTransitiveDeps in src/jsifier.mjs. NFC #28054

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sbc100 wants to merge 2 commits into from
Closed

Memoize getTransitiveDeps in src/jsifier.mjs. NFC #28054

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ffd5abc
Calculate transitive JS library deps after `symbolHandler` in `--symb…
sbc100 Oct 22, 2026
8546616
Memoize `getTransitiveDeps` in `src/jsifier.mjs`. NFC
sbc100 Oct 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 30 additions & 15 deletions src/jsifier.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,27 +127,40 @@ function isDefined(symName) {
return false;
}

const transitiveDepsCache = new Map();

@gemini-code-assist gemini-code-assist Bot Oct 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Since runJSify can be called multiple times within the same Node.js process (for example, in test suites or in-process build pipelines), a module-level global Map like transitiveDepsCache will persist across different compilations. This can lead to stale cache entries and incorrect dependency resolution if different compilations use different libraries or settings.

To prevent this cross-compilation contamination, transitiveDepsCache should be cleared at the start of runJSify:

export async function runJSify(outputFile, symbolsOnly) {
  transitiveDepsCache.clear();
  ...


function getTransitiveDeps(symbol) {
// TODO(sbc): Use some kind of cache to avoid quadratic behaviour here.
let result = transitiveDepsCache.get(symbol);
if (result) {
return result;
}
const transitiveDeps = new Set();
const seen = new Set();
const toVisit = [symbol];
while (toVisit.length) {
const sym = toVisit.pop();
if (!seen.has(sym)) {
let directDeps = LibraryManager.library[sym + '__deps'] ?? [];
directDeps = directDeps.filter((d) => typeof d === 'string');
for (const dep of directDeps) {
const cached = transitiveDepsCache.get(sym);
let deps = cached;
if (!deps) {
deps = LibraryManager.library[sym + '__deps'] ?? [];
deps = deps.filter((d) => typeof d === 'string');
}
Comment on lines +143 to +148

@gemini-code-assist gemini-code-assist Bot Oct 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This block can be simplified and made more idiomatic by using the nullish coalescing operator (??) and declaring deps as a const.

      const cached = transitiveDepsCache.get(sym);
      const deps = cached ?? (LibraryManager.library[sym + '__deps'] ?? [])
        .filter((d) => typeof d === 'string');

for (const dep of deps) {
if (!transitiveDeps.has(dep)) {
debugLog(`adding dependency ${symbol} -> ${dep}`);
transitiveDeps.add(dep);
if (!cached) {
toVisit.push(dep);
}
}
transitiveDeps.add(dep);
toVisit.push(dep);
}
seen.add(sym);
}
}
return Array.from(transitiveDeps);
result = Array.from(transitiveDeps);
transitiveDepsCache.set(symbol, result);
return result;
}

function shouldPreprocess(fileName) {
Expand Down Expand Up @@ -413,9 +426,9 @@ const PROXY_SYNC = 1;
const PROXY_SYNC_ASYNC = 2;

export async function runJSify(outputFile, symbolsOnly) {
transitiveDepsCache.clear();
const libraryItems = [];
const libraryAliases = [];
const symbolDeps = {};
const asyncFuncs = [];
let postSets = [];

Expand Down Expand Up @@ -629,13 +642,6 @@ function(${args}) {
}

if (symbolsOnly) {
if (LibraryManager.library.hasOwnProperty(symbol)) {
// Resolve aliases before looking up deps
var transitiveDeps = getTransitiveDeps(symbol);
symbolDeps[symbol] = transitiveDeps.filter(
(d) => !isJsOnlySymbol(d) && !(d in LibraryManager.library),
);
}
return;
}

Expand Down Expand Up @@ -1003,6 +1009,15 @@ var proxiedFunctionTable = [
timer.stop('symbolHandler');

if (symbolsOnly) {
const symbolDeps = {};
for (const sym of Object.keys(LibraryManager.library)) {
if (!isDecorator(sym)) {
const transitiveDeps = getTransitiveDeps(sym);
symbolDeps[sym] = transitiveDeps.filter(
(d) => !isJsOnlySymbol(d) && !(d in LibraryManager.library),
);
}
}
writeOutput(
JSON.stringify({
deps: symbolDeps,
Expand Down
Loading