Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions apps/admin/app/entry.client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@ import { startTransition, StrictMode } from "react";
import { hydrateRoot } from "react-dom/client";
import { HydratedRouter } from "react-router/dom";

if (typeof window !== "undefined" && typeof window.requestIdleCallback !== "function") {
window.requestIdleCallback = (cb: any) => {
const start = Date.now();
return setTimeout(() => {
cb({
didTimeout: false,
timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
});
}, 1) as any;
};
window.cancelIdleCallback = (id: any) => clearTimeout(id);
}
Comment on lines +11 to +22

Comment on lines +11 to +23

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Duplicated polyfill logic + loose typing (any).

This block is copy-pasted verbatim across apps/admin, apps/space, and apps/web entry files, and duplicates the already-existing installIdleCallbackPolyfill in apps/web/core/lib/idle-task.ts. Three independent copies of the same fallback logic will drift over time (e.g., one uses window, the other globalThis; timeout/timeRemaining constants could diverge). Also, cb: any and id: any violate the repo's strict-typing guideline.

Consider extracting this into a shared, typed polyfill module (e.g. a workspace:* package) and importing it from all three entry points, reusing the IdleRequestCallback/IdleRequestOptions types already established in apps/web/core/lib/idle-task.ts.

♻️ Suggested typed version
-if (typeof window !== "undefined" && typeof window.requestIdleCallback !== "function") {
-  window.requestIdleCallback = (cb: any) => {
-    const start = Date.now();
-    return setTimeout(() => {
-      cb({
-        didTimeout: false,
-        timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
-      });
-    }, 1) as any;
-  };
-  window.cancelIdleCallback = (id: any) => clearTimeout(id);
-}
+if (typeof window !== "undefined" && typeof window.requestIdleCallback !== "function") {
+  window.requestIdleCallback = (cb: IdleRequestCallback, options?: IdleRequestOptions): number => {
+    const start = Date.now();
+    return window.setTimeout(() => {
+      cb({
+        didTimeout: false,
+        timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
+      });
+    }, options?.timeout ?? 1);
+  };
+  window.cancelIdleCallback = (id: number) => window.clearTimeout(id);
+}

As per coding guidelines, **/*.{ts,tsx}: "TypeScript strict mode enabled; all files must be typed."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (typeof window !== "undefined" && typeof window.requestIdleCallback !== "function") {
window.requestIdleCallback = (cb: any) => {
const start = Date.now();
return setTimeout(() => {
cb({
didTimeout: false,
timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
});
}, 1) as any;
};
window.cancelIdleCallback = (id: any) => clearTimeout(id);
}
if (typeof window !== "undefined" && typeof window.requestIdleCallback !== "function") {
window.requestIdleCallback = (cb: IdleRequestCallback, options?: IdleRequestOptions): number => {
const start = Date.now();
return window.setTimeout(() => {
cb({
didTimeout: false,
timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
});
}, options?.timeout ?? 1);
};
window.cancelIdleCallback = (id: number) => window.clearTimeout(id);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/admin/app/entry.client.tsx` around lines 11 - 23, The idle-callback
fallback in the entry client is duplicated and uses loose `any` types, so
replace it with the shared typed polyfill used by installIdleCallbackPolyfill in
apps/web/core/lib/idle-task.ts. Extract or reuse a common module for the
requestIdleCallback/cancelIdleCallback setup, and import it from the
admin/space/web entry points so there is one source of truth. Make sure the
shared helper uses the existing IdleRequestCallback and IdleRequestOptions types
instead of any, and keep the entry file limited to invoking that helper.

Source: Coding guidelines

startTransition(() => {
hydrateRoot(
document,
Expand Down
13 changes: 13 additions & 0 deletions apps/space/app/entry.client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@ import { startTransition, StrictMode } from "react";
import { hydrateRoot } from "react-dom/client";
import { HydratedRouter } from "react-router/dom";

if (typeof window !== "undefined" && typeof window.requestIdleCallback !== "function") {
window.requestIdleCallback = (cb: any) => {
const start = Date.now();
return setTimeout(() => {
cb({
didTimeout: false,
timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
});
}, 1) as any;
Comment on lines +12 to +19

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Fallback ignores the options.timeout param.

Native requestIdleCallback accepts (callback, options); this fallback only accepts cb, so any caller passing { timeout: N } silently loses that hint and always fires after 1ms. This diverges from the sibling implementation in apps/web/core/lib/idle-task.ts, which honors options?.timeout.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/space/app/entry.client.tsx` around lines 12 - 19, The
requestIdleCallback fallback in entry.client.tsx ignores the optional timeout
argument, so callers passing options lose the intended behavior. Update the
fallback signature to accept both the callback and options, and use the provided
timeout when scheduling the setTimeout fallback, matching the behavior in the
sibling idle-task implementation. Keep the existing cb wrapper and timeRemaining
logic, but ensure options?.timeout is honored instead of always using 1ms.

};
window.cancelIdleCallback = (id: any) => clearTimeout(id);
}
Comment on lines +11 to +22
Comment on lines +11 to +22

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Duplicated polyfill; consider a shared utility and stricter typing.

This same block is copy-pasted verbatim across admin/space/web entry files, and apps/web/core/lib/idle-task.ts already implements an equivalent installIdleCallbackPolyfill (with options.timeout support). Extracting a single shared implementation (e.g., a workspace package) avoids drift between the three copies and reuses the existing, more complete implementation. Also, cb: any / id: any / as any bypass strict typing — use IdleRequestCallback/IdleRequestOptions/number instead.

♻️ Suggested typed fix
-if (typeof window !== "undefined" && typeof window.requestIdleCallback !== "function") {
-  window.requestIdleCallback = (cb: any) => {
-    const start = Date.now();
-    return setTimeout(() => {
-      cb({
-        didTimeout: false,
-        timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
-      });
-    }, 1) as any;
-  };
-  window.cancelIdleCallback = (id: any) => clearTimeout(id);
-}
+if (typeof window !== "undefined" && typeof window.requestIdleCallback !== "function") {
+  window.requestIdleCallback = (cb: IdleRequestCallback, options?: IdleRequestOptions) => {
+    const start = Date.now();
+    return setTimeout(() => {
+      cb({
+        didTimeout: false,
+        timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
+      });
+    }, options?.timeout ?? 1) as unknown as number;
+  };
+  window.cancelIdleCallback = (id: number) => clearTimeout(id);
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (typeof window !== "undefined" && typeof window.requestIdleCallback !== "function") {
window.requestIdleCallback = (cb: any) => {
const start = Date.now();
return setTimeout(() => {
cb({
didTimeout: false,
timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
});
}, 1) as any;
};
window.cancelIdleCallback = (id: any) => clearTimeout(id);
}
if (typeof window !== "undefined" && typeof window.requestIdleCallback !== "function") {
window.requestIdleCallback = (cb: IdleRequestCallback, options?: IdleRequestOptions) => {
const start = Date.now();
return setTimeout(() => {
cb({
didTimeout: false,
timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
});
}, options?.timeout ?? 1) as unknown as number;
};
window.cancelIdleCallback = (id: number) => clearTimeout(id);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/space/app/entry.client.tsx` around lines 11 - 22, The
requestIdleCallback polyfill in entry.client.tsx is duplicated across entry
files and also overlaps with the existing installIdleCallbackPolyfill utility in
apps/web/core/lib/idle-task.ts. Replace the inline block with a shared import so
all entry points reuse the same implementation, including the options.timeout
behavior already supported there. While updating the shared polyfill, remove the
any casts by typing the callback and cancel handle with IdleRequestCallback,
IdleRequestOptions, and number to keep strict typing intact.

Source: Coding guidelines


startTransition(() => {
hydrateRoot(
document,
Expand Down
12 changes: 12 additions & 0 deletions apps/web/app/entry.client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,18 @@ import polyfills from "@/lib/polyfills";

void polyfills;

if (typeof window !== "undefined" && typeof window.requestIdleCallback !== "function") {
window.requestIdleCallback = (cb: any) => {
const start = Date.now();
return setTimeout(() => {
cb({
didTimeout: false,
timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
});
}, 1) as any;
Comment on lines +16 to +23

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Same any-typing / dropped options.timeout concerns as the space app entry.

cb: any/id: any/as any bypass strict typing, and the fallback signature drops the options parameter that native requestIdleCallback supports, silently overriding any requested timeout with a hardcoded 1ms delay.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/entry.client.tsx` around lines 16 - 23, The requestIdleCallback
polyfill in entry.client.tsx is using any-typed parameters and a hardcoded
fallback that ignores the native options timeout. Update the
window.requestIdleCallback shim to use proper requestIdleCallback-style types
instead of cb: any/id: any/as any, and accept the optional options parameter so
a caller-provided timeout is preserved rather than always forcing a 1ms delay.

};
window.cancelIdleCallback = (id: any) => clearTimeout(id);
}
Comment on lines +15 to +26
Comment on lines +15 to +26

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Redundant with existing installIdleCallbackPolyfill; prefer reusing it.

apps/web/core/lib/idle-task.ts already exports installIdleCallbackPolyfill, invoked via apps/web/core/lib/polyfills/index.ts on import, and it already supports options.timeout via requestIdleFallback. Adding a second, slightly different inline implementation here (using window instead of globalThis, dropping options, and typed with any) creates two competing polyfill paths in the same app. Prefer importing and calling the existing installer instead of duplicating the logic.

♻️ Suggested fix
-if (typeof window !== "undefined" && typeof window.requestIdleCallback !== "function") {
-  window.requestIdleCallback = (cb: any) => {
-    const start = Date.now();
-    return setTimeout(() => {
-      cb({
-        didTimeout: false,
-        timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
-      });
-    }, 1) as any;
-  };
-  window.cancelIdleCallback = (id: any) => clearTimeout(id);
-}
+import { installIdleCallbackPolyfill } from "`@/lib/idle-task`";
+
+installIdleCallbackPolyfill();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (typeof window !== "undefined" && typeof window.requestIdleCallback !== "function") {
window.requestIdleCallback = (cb: any) => {
const start = Date.now();
return setTimeout(() => {
cb({
didTimeout: false,
timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
});
}, 1) as any;
};
window.cancelIdleCallback = (id: any) => clearTimeout(id);
}
import { installIdleCallbackPolyfill } from "`@/lib/idle-task`";
installIdleCallbackPolyfill();
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/entry.client.tsx` around lines 15 - 26, `entry.client.tsx` is
duplicating the idle callback polyfill that already exists in
`installIdleCallbackPolyfill`. Remove the inline
`requestIdleCallback`/`cancelIdleCallback` shim and reuse the shared installer
from `idle-task.ts` (through the existing polyfills entrypoint), so there is a
single implementation that preserves `options.timeout` and the
`globalThis`-based behavior.

startTransition(() => {
hydrateRoot(
document,
Expand Down