diff --git a/.github/workflows/pr-actions.yml b/.github/workflows/pr-actions.yml index c88a3d7967..08a3bb3d31 100644 --- a/.github/workflows/pr-actions.yml +++ b/.github/workflows/pr-actions.yml @@ -50,4 +50,8 @@ jobs: - name: Apply actions env: GITHUB_TOKEN: ${{ github.token }} + WORKFLOW_RUN_HEAD_OWNER: ${{ github.event.workflow_run.head_repository.owner.login }} + WORKFLOW_RUN_HEAD_REPO_ID: ${{ github.event.workflow_run.head_repository.id }} + WORKFLOW_RUN_HEAD_BRANCH: ${{ github.event.workflow_run.head_branch }} + WORKFLOW_RUN_HEAD_SHA: ${{ github.event.workflow_run.head_sha }} run: node scripts/pr.ts actions pr-checks-result.json diff --git a/.github/workflows/preview.yml b/.github/workflows/preview.yml index 24f26ecf21..805f0da838 100644 --- a/.github/workflows/preview.yml +++ b/.github/workflows/preview.yml @@ -95,9 +95,12 @@ jobs: # pushes over remote branches in case of a branch name collision - name: Build/push branch (workflow_dispatch) if: github.event_name == 'workflow_dispatch' + env: + INSTALLABLE_BRANCH: ${{ inputs.installableBranch }} run: | - node ./scripts/previews/branch.ts ${{ inputs.installableBranch }} - git push --set-upstream origin ${{ inputs.installableBranch }} + git check-ref-format --branch "$INSTALLABLE_BRANCH" + node ./scripts/previews/branch.ts "$INSTALLABLE_BRANCH" + git push --set-upstream origin "$INSTALLABLE_BRANCH" echo "💿 pushed installable branch: https://github.com/$GITHUB_REPOSITORY/commit/$(git rev-parse HEAD)" # Cleanup PR preview/pr-{number} branches when the PR is closed diff --git a/.github/workflows/release-comments-manual.yml b/.github/workflows/release-comments-manual.yml index 62d8db5228..ef4db8df32 100644 --- a/.github/workflows/release-comments-manual.yml +++ b/.github/workflows/release-comments-manual.yml @@ -39,4 +39,5 @@ jobs: - name: Comment on released issues and pull requests env: GH_TOKEN: ${{ github.token }} - run: pnpm run release-comments --release=${{ github.event.inputs.release }} + RELEASE: ${{ inputs.release }} + run: pnpm run release-comments --release="$RELEASE" diff --git a/contributors.yml b/contributors.yml index a399fdfb45..008b3b8911 100644 --- a/contributors.yml +++ b/contributors.yml @@ -109,6 +109,7 @@ - danielberndt - danielweinmann - daniilguit +- Dante-Cho - dauletbaev - david-bezero - david-crespo diff --git a/packages/react-router/.changes/patch.scroll-restoration-bfcache.md b/packages/react-router/.changes/patch.scroll-restoration-bfcache.md new file mode 100644 index 0000000000..c21df5e8bd --- /dev/null +++ b/packages/react-router/.changes/patch.scroll-restoration-bfcache.md @@ -0,0 +1 @@ +Fix `` leaving `history.scrollRestoration` set to `"auto"` after a bfcache restore, which let the browser restore scroll on subsequent history traversals before the destination route had rendered diff --git a/packages/react-router/__tests__/dom/scroll-restoration-test.tsx b/packages/react-router/__tests__/dom/scroll-restoration-test.tsx index f6ee4113a3..7ad7ac05c3 100644 --- a/packages/react-router/__tests__/dom/scroll-restoration-test.tsx +++ b/packages/react-router/__tests__/dom/scroll-restoration-test.tsx @@ -192,6 +192,55 @@ describe(`ScrollRestoration`, () => { consoleWarnMock.mockRestore(); }); + it("re-enables manual scroll restoration when the page is restored from the bfcache", () => { + jest.restoreAllMocks(); + + let testWindow = getWindow("/base"); + window.scrollTo = jest.fn(); + + let router = createBrowserRouter( + [ + { + path: "/", + Component() { + return ( + <> + + + + ); + }, + children: testPages, + }, + ], + { basename: "/base", window: testWindow }, + ); + let { unmount } = render(); + + // While is mounted we own scroll restoration + expect(window.history.scrollRestoration).toBe("manual"); + + // Hand it back to the browser when the page is hidden, so that a + // re-created document restores its own scroll position + window.dispatchEvent(new Event("pagehide")); + expect(window.history.scrollRestoration).toBe("auto"); + + // A discarded document goes through mount again, so we leave that case + // to the browser + window.dispatchEvent(pageShowEvent(false)); + expect(window.history.scrollRestoration).toBe("auto"); + + // A bfcache restore keeps this document (and this component) alive, so we + // own scroll restoration again + window.dispatchEvent(pageShowEvent(true)); + expect(window.history.scrollRestoration).toBe("manual"); + + // …until we're unmounted, after which the browser keeps control + unmount(); + window.dispatchEvent(pageShowEvent(true)); + expect(window.history.scrollRestoration).toBe("auto"); + }); + describe("SSR", () => { let scrollTo = window.scrollTo; beforeAll(() => { @@ -377,6 +426,12 @@ describe(`ScrollRestoration`, () => { }); }); +function pageShowEvent(persisted: boolean) { + let event = new Event("pageshow"); + Object.defineProperty(event, "persisted", { value: persisted }); + return event; +} + const testPages = [ { index: true, diff --git a/packages/react-router/lib/dom/lib.tsx b/packages/react-router/lib/dom/lib.tsx index f779abea62..32d5d93de6 100644 --- a/packages/react-router/lib/dom/lib.tsx +++ b/packages/react-router/lib/dom/lib.tsx @@ -3106,6 +3106,15 @@ export function useScrollRestoration({ }; }, []); + // Re-enable manual scroll restoration on a bfcache restore + usePageShow( + React.useCallback((event: PageTransitionEvent) => { + if (event.persisted) { + window.history.scrollRestoration = "manual"; + } + }, []), + ); + // Save positions on pagehide usePageHide( React.useCallback(() => { @@ -3251,6 +3260,24 @@ function usePageHide( }, [callback, capture]); } +/* + * Setup a callback to be fired on the window's `pageshow` event. The event's + * `persisted` flag indicates the document was restored from the bfcache. + */ +function usePageShow( + callback: (event: PageTransitionEvent) => any, + options?: { capture?: boolean }, +): void { + let { capture } = options || {}; + React.useEffect(() => { + let opts = capture != null ? { capture } : undefined; + window.addEventListener("pageshow", callback, opts); + return () => { + window.removeEventListener("pageshow", callback, opts); + }; + }, [callback, capture]); +} + /** * Wrapper around {@link useBlocker} to show a [`window.confirm`](https://developer.mozilla.org/en-US/docs/Web/API/Window/confirm) * prompt to users instead of building a custom UI with {@link useBlocker}. diff --git a/scripts/pr.ts b/scripts/pr.ts index 2a505670f7..6aee76fb08 100644 --- a/scripts/pr.ts +++ b/scripts/pr.ts @@ -28,6 +28,10 @@ * * Environment (actions): * GITHUB_TOKEN - Required (issues:write + pull-requests:write). + * WORKFLOW_RUN_HEAD_OWNER - Required. workflow_run.head_repository.owner.login + * WORKFLOW_RUN_HEAD_REPO_ID - Required. workflow_run.head_repository.id + * WORKFLOW_RUN_HEAD_BRANCH - Required. workflow_run.head_branch + * WORKFLOW_RUN_HEAD_SHA - Required. workflow_run.head_sha */ import * as fs from "node:fs"; import * as util from "node:util"; @@ -39,6 +43,7 @@ import { createPrComment, getPrComments, getPrFiles, + getWorkflowRunPrNumber, removePrLabel, updatePrComment, } from "./utils/github.ts"; @@ -344,7 +349,9 @@ async function runActions() { return; } - let { prNumber, actions } = JSON.parse(fs.readFileSync(filename, "utf8")) as { + let { prNumber: artifactPrNumber, actions } = JSON.parse( + fs.readFileSync(filename, "utf8"), + ) as { prNumber: number; actions: Action[]; }; @@ -354,6 +361,25 @@ async function runActions() { return; } + let headRepositoryId = Number(requireEnv("WORKFLOW_RUN_HEAD_REPO_ID")); + if (!Number.isSafeInteger(headRepositoryId) || headRepositoryId <= 0) { + throw new Error("WORKFLOW_RUN_HEAD_REPO_ID must be a positive integer"); + } + + // The artifact is PR-controlled. Resolve its only permitted target from the + // trusted workflow_run event and reject stale or cross-PR instructions. + let prNumber = await getWorkflowRunPrNumber({ + headOwner: requireEnv("WORKFLOW_RUN_HEAD_OWNER"), + headRepositoryId, + headBranch: requireEnv("WORKFLOW_RUN_HEAD_BRANCH"), + headSha: requireEnv("WORKFLOW_RUN_HEAD_SHA"), + }); + if (artifactPrNumber !== prNumber) { + throw new Error( + `Artifact targets PR #${String(artifactPrNumber)}, but workflow run belongs to PR #${prNumber}`, + ); + } + console.log(actions); for (let action of actions) { diff --git a/scripts/previews/branch.ts b/scripts/previews/branch.ts index 0344272b59..aab335224d 100644 --- a/scripts/previews/branch.ts +++ b/scripts/previews/branch.ts @@ -55,7 +55,7 @@ async function main() { ); // Switch to new branch and reset to current commit on base branch - logAndExec(`git checkout -B ${installableBranch}`); + logAndExec(["git", "checkout", "-B", installableBranch]); // Build dist/ folders logAndExec("pnpm build"); diff --git a/scripts/utils/github.ts b/scripts/utils/github.ts index 7cbb4138d9..deeb30b40c 100644 --- a/scripts/utils/github.ts +++ b/scripts/utils/github.ts @@ -5,6 +5,13 @@ import { getGitTag } from "./packages.ts"; const OWNER = "remix-run"; const REPO = "react-router"; +type WorkflowRunHead = { + headOwner: string; + headRepositoryId: number; + headBranch: string; + headSha: string; +}; + function getToken(): string { let token = process.env.GITHUB_TOKEN; if (!token) { @@ -119,6 +126,32 @@ export async function findOpenPr(head: string, base: string) { return response.data.length > 0 ? response.data[0] : null; } +/** + * Resolve the open PR whose current head produced a workflow_run event. + */ +export async function getWorkflowRunPrNumber(workflowRun: WorkflowRunHead) { + let response = await request("GET /repos/{owner}/{repo}/pulls", { + ...requestOptions(), + state: "open", + head: `${workflowRun.headOwner}:${workflowRun.headBranch}`, + per_page: 100, + }); + + let matches = response.data.filter( + (pr) => + pr.head.repo?.id === workflowRun.headRepositoryId && + pr.head.ref === workflowRun.headBranch && + pr.head.sha === workflowRun.headSha, + ); + if (matches.length !== 1) { + throw new Error( + `Expected exactly one open PR for workflow run, found ${matches.length}`, + ); + } + + return matches[0].number; +} + /** * Create a new PR */