mirror of
https://github.com/makeplane/plane.git
synced 2026-08-29 10:08:51 +02:00
fix: resolve React Doctor errors and restore its PR baseline (#9488)
* fix: resolve React Doctor errors and restore its PR baseline The React Doctor check on PR #9160 reported 266 issues across 130 files. Most of that is a reporting artifact: the workflow's `actions/checkout` step used the default shallow clone, so React Doctor had no merge base to diff against and fell back to listing every pre-existing issue in every changed file rather than only what the PR introduced. Add `fetch-depth: 0` so the comparison works. Also point the `push` trigger at `preview` — the repo's default branch — instead of `main`, which does not exist here, so the health-score trend never ran. Fix the 7 genuine errors it surfaced: - use-keypress: `callback` sat in the effect deps while every one of the 10 call sites passes an inline arrow, so the document listener was torn down and re-added on every render. Latch the callback in a ref and key the subscription on `key` alone. This clears `no-effect-with-fresh-deps` at create-root.tsx:102 and create-project-modal.tsx:64 at the source. - estimates/points/preview: the dblclick listener was added with no cleanup, so listeners accumulated on every toggle. Add the matching removeEventListener. - issues/header, calendar/issue-block, pages/editor/editor-body: guard `window` reads that run during render with `typeof window !== "undefined"`, matching the pattern already used elsewhere in web and admin. The editor case previously relied on the surrounding try/catch swallowing a ReferenceError on the server. Also clears the five pre-existing oxlint warnings in calendar/issue-block that the repo's `--deny-warnings` pre-commit gate blocks on once the file is touched: rename a shadowed `issue` parameter, and mark two presentational wrapper divs with `role="presentation"` — CustomMenu already wraps the first in a real <button>, and the second exists only to stop click propagation to the surrounding ControlLink. Verified: `turbo run check:types --filter=web` passes (11/11 tasks), `oxlint --deny-warnings` reports 0 warnings and 0 errors on the changed files. Claude-Session: https://claude.ai/code/session_01Hrr1nfNBiyC256drHM8BbT * fix: import EditorAIMenu directly instead of through the ai barrel The only issue React Doctor reports against this branch. `./ai` re-exports both menu.tsx and ask-pi-menu.tsx, so importing through it pulls the ask-pi menu into the page editor bundle for a symbol that lives in menu.tsx. Pre-existing on preview rather than introduced here, but it is a one-line fix in a file this branch already touches. Claude-Session: https://claude.ai/code/session_01Hrr1nfNBiyC256drHM8BbT * fix: address CodeRabbit review on the React Doctor branch - react-doctor.yml: set persist-credentials: false on checkout. The token otherwise stays in .git/config for the third-party millionco/react-doctor step that runs next. fetch-depth: 0 has already fetched every ref the merge-base diff needs, and the action authenticates to the API through its own credentials, so nothing depends on the persisted git credential. - calendar/issue-block: pass workspaceSlug?.toString() to handleRedirection. The param is typed string | undefined and line 89 of the same file already optional-chains it; this call site would have thrown on a missing route param. Pre-existing, but it is on a line this branch already touches. Declined the two SSR/hydration findings: apps/web sets ssr: false in react-router.config.ts and ships a client-only bundle, so there is no server render to diverge from. The typeof window guards satisfy the React Doctor rule but are defensive only. Claude-Session: https://claude.ai/code/session_01Hrr1nfNBiyC256drHM8BbT
This commit is contained in:
committed by
GitHub
parent
08a7d12b9d
commit
7564480cf7
14
.github/workflows/react-doctor.yml
vendored
14
.github/workflows/react-doctor.yml
vendored
@@ -9,13 +9,13 @@ name: React Doctor
|
||||
on:
|
||||
pull_request:
|
||||
types: [opened, synchronize, reopened, ready_for_review]
|
||||
# Scans `main` on every push so you get a health-score trend on the
|
||||
# Scans `preview` on every push so you get a health-score trend on the
|
||||
# default branch — useful for tracking the overall number commit-by-commit
|
||||
# and catching regressions that slipped past PR review. PR-specific steps
|
||||
# (the sticky summary comment) are skipped automatically on `push` events.
|
||||
# Comment this block out if you only want PR-time scans.
|
||||
push:
|
||||
branches: [main]
|
||||
branches: [preview]
|
||||
|
||||
permissions:
|
||||
# `actions/checkout` needs this to read the repo source.
|
||||
@@ -47,6 +47,16 @@ jobs:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v5
|
||||
with:
|
||||
# React Doctor diffs the changed files against the merge base so it
|
||||
# only reports what the PR introduces. The default shallow checkout
|
||||
# has no merge base, which makes it fall back to listing every
|
||||
# pre-existing issue in every changed file.
|
||||
fetch-depth: 0
|
||||
# Don't leave the workflow token in .git/config for the third-party
|
||||
# action that runs next. fetch-depth: 0 has already fetched every ref
|
||||
# it needs, and it talks to the API through its own credentials.
|
||||
persist-credentials: false
|
||||
|
||||
- uses: millionco/react-doctor@v2
|
||||
# Common configuration knobs — uncomment any to override the default.
|
||||
|
||||
@@ -53,8 +53,15 @@ export const EstimatePointItemPreview = observer(function EstimatePointItemPrevi
|
||||
const EstimatePointValueRef = useRef<HTMLDivElement>(null);
|
||||
|
||||
useEffect(() => {
|
||||
if (!estimatePointEditToggle && !estimatePointDeleteToggle)
|
||||
EstimatePointValueRef?.current?.addEventListener("dblclick", () => setEstimatePointEditToggle(true));
|
||||
const estimatePointValueElement = EstimatePointValueRef.current;
|
||||
if (!estimatePointValueElement || estimatePointEditToggle || estimatePointDeleteToggle) return;
|
||||
|
||||
const handleDoubleClick = () => setEstimatePointEditToggle(true);
|
||||
estimatePointValueElement.addEventListener("dblclick", handleDoubleClick);
|
||||
|
||||
return () => {
|
||||
estimatePointValueElement.removeEventListener("dblclick", handleDoubleClick);
|
||||
};
|
||||
}, [estimatePointDeleteToggle, estimatePointEditToggle]);
|
||||
|
||||
return (
|
||||
|
||||
@@ -55,7 +55,9 @@ export const IssuesHeader = observer(function IssuesHeader() {
|
||||
const { allowPermissions } = useUserPermissions();
|
||||
const { isMobile } = usePlatformOS();
|
||||
|
||||
const SPACE_APP_URL = (SPACE_BASE_URL.trim() === "" ? window.location.origin : SPACE_BASE_URL) + SPACE_BASE_PATH;
|
||||
const SPACE_APP_URL =
|
||||
(SPACE_BASE_URL.trim() === "" && typeof window !== "undefined" ? window.location.origin : SPACE_BASE_URL) +
|
||||
SPACE_BASE_PATH;
|
||||
const publishedURL = `${SPACE_APP_URL}/issues/${currentProjectDetails?.anchor}`;
|
||||
|
||||
const issuesCount = getGroupIssueCount(undefined, undefined, false);
|
||||
|
||||
@@ -58,12 +58,16 @@ export const CalendarIssueBlock = observer(
|
||||
const projectIdentifier = getProjectIdentifierById(issue?.project_id);
|
||||
|
||||
// handlers
|
||||
const handleIssuePeekOverview = (issue: TIssue) => handleRedirection(workspaceSlug.toString(), issue, isMobile);
|
||||
const handleIssuePeekOverview = (peekIssue: TIssue) =>
|
||||
handleRedirection(workspaceSlug?.toString(), peekIssue, isMobile);
|
||||
|
||||
useOutsideClickDetector(menuActionRef, () => setIsMenuActive(false));
|
||||
|
||||
const customActionButton = (
|
||||
// CustomMenu renders this inside its own <button>, which already carries the
|
||||
// interactive semantics and keyboard handling — this div is presentational.
|
||||
<div
|
||||
role="presentation"
|
||||
ref={menuActionRef}
|
||||
className={`w-full cursor-pointer rounded-sm p-1 text-placeholder hover:bg-layer-1 ${
|
||||
isMenuActive ? "bg-layer-1-active text-primary" : "text-secondary"
|
||||
@@ -75,7 +79,9 @@ export const CalendarIssueBlock = observer(
|
||||
);
|
||||
|
||||
const isMenuActionRefAboveScreenBottom =
|
||||
menuActionRef?.current && menuActionRef?.current?.getBoundingClientRect().bottom < window.innerHeight - 220;
|
||||
typeof window !== "undefined" &&
|
||||
menuActionRef?.current &&
|
||||
menuActionRef?.current?.getBoundingClientRect().bottom < window.innerHeight - 220;
|
||||
|
||||
const placement = isMenuActionRefAboveScreenBottom ? "bottom-end" : "top-end";
|
||||
|
||||
@@ -136,7 +142,10 @@ export const CalendarIssueBlock = observer(
|
||||
)}
|
||||
<div className="truncate text-13 font-medium md:text-11 md:font-regular">{issue.name}</div>
|
||||
</div>
|
||||
{/* Wrapper exists only to stop clicks reaching the ControlLink; the
|
||||
quick-action menu inside carries its own interactive semantics. */}
|
||||
<div
|
||||
role="presentation"
|
||||
className={cn("size-5 flex-shrink-0", {
|
||||
"hidden group-hover/calendar-block:block": !isMobile,
|
||||
block: isMenuActive,
|
||||
|
||||
@@ -44,7 +44,7 @@ import type { TPageInstance } from "@/store/pages/base-page";
|
||||
import { PageContentLoader } from "../loaders/page-content-loader";
|
||||
import { PageEditorHeaderRoot } from "./header";
|
||||
import { PageContentBrowser } from "./summary";
|
||||
import { EditorAIMenu } from "./ai";
|
||||
import { EditorAIMenu } from "./ai/menu";
|
||||
|
||||
export type TEditorBodyConfig = {
|
||||
fileHandler: TFileHandler;
|
||||
@@ -188,6 +188,11 @@ export const PageEditorBody = observer(function PageEditorBody(props: Props) {
|
||||
);
|
||||
|
||||
const realtimeConfig: TRealtimeConfig | undefined = useMemo(() => {
|
||||
// The collaboration URL is derived from window.location, so it can only be
|
||||
// built on the client. There is no socket to connect to during SSR anyway —
|
||||
// the config is recomputed on hydration.
|
||||
if (typeof window === "undefined") return undefined;
|
||||
|
||||
// Construct the WebSocket Collaboration URL
|
||||
try {
|
||||
const LIVE_SERVER_BASE_URL = LIVE_BASE_URL?.trim() || window.location.origin;
|
||||
|
||||
@@ -4,13 +4,21 @@
|
||||
* See the LICENSE file for details.
|
||||
*/
|
||||
|
||||
import { useEffect } from "react";
|
||||
import { useEffect, useRef } from "react";
|
||||
|
||||
const useKeypress = (key: string, callback: (event: KeyboardEvent) => void) => {
|
||||
// Keep the latest callback in a ref so callers can pass an inline arrow
|
||||
// without tearing down and re-adding the document listener every render.
|
||||
const callbackRef = useRef(callback);
|
||||
|
||||
useEffect(() => {
|
||||
callbackRef.current = callback;
|
||||
}, [callback]);
|
||||
|
||||
useEffect(() => {
|
||||
const handleKeydown = (event: KeyboardEvent) => {
|
||||
if (event.key === key) {
|
||||
callback(event);
|
||||
callbackRef.current(event);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -19,7 +27,7 @@ const useKeypress = (key: string, callback: (event: KeyboardEvent) => void) => {
|
||||
return () => {
|
||||
document.removeEventListener("keydown", handleKeydown);
|
||||
};
|
||||
}, [key, callback]);
|
||||
}, [key]);
|
||||
};
|
||||
|
||||
export default useKeypress;
|
||||
|
||||
Reference in New Issue
Block a user