-
-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Repo housekeeping updates #13880
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.
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
Repo housekeeping updates #13880
Changes from all commits
66448f5
e8d752e
281635c
ecaf584
54718fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,46 +8,35 @@ body: | |
value: | | ||
Thank you for helping to improve React Router! | ||
|
||
- **All** bugs must have a **minimal** reproduction | ||
- Minimal means that it is not just pointing to a deployed site or a branch in your existing application | ||
- The preferred method is StackBlitz via [https://reactrouter.com/new](https://reactrouter.com/new) | ||
- If Stackblitz is not an option, a GitHub repo based on a fresh `create-react-router` app is acceptable | ||
- Only in extraordinary circumstances will code snippets or maximal reproductions be accepted | ||
- Issue Review | ||
- Issues not meeting the above criteria will be closed and pointed to this document | ||
- Non-issues (feature requests, usage questions) will also be closed with a link to this document | ||
- The SC will triage issues regularly | ||
- Fixing Issues | ||
- The SC will mark good community issues with an `Accepting PRs` label | ||
- These issues will generally be ones that are likely to have a small surface area fix | ||
- However, anyone can work on any issue but there's no guarantee the PR will be accepted if the surface area is too large for expedient review by a core team member | ||
Please note that **all** bugs must have a **minimal** and **runnable** | ||
reproduction, meaning that it is not just pointing to a deployed site or | ||
a branch in your existing application, or showing a small code snippet. | ||
For more information, please refer to the | ||
[Bug/Issue Process Guidelines](https://github.com/remix-run/react-router/blob/main/GOVERNANCE.md#bugissue-process). | ||
|
||
## Option 1: Submit a PR with a failing test | ||
## If you are using **Framework Mode**, you have 2 preferred options: | ||
|
||
**Option 1** - Create a failing integration test | ||
|
||
🏆 The most helpful reproduction is to use our _bug report integration test_ template: | ||
|
||
1. [Fork `remix-run/react-router`](https://github.com/remix-run/react-router/fork) | ||
2. Open [`integration/bug-report-test.ts`](https://github.com/remix-run/react-router/blob/dev/integration/bug-report-test.ts) in your editor | ||
3. Follow the instructions and submit a pull request with a failing bug report test! | ||
|
||
## Option 2: Continue filling out this form | ||
3. Follow the instructions to create a failing bug report test | ||
4. Link to your forked branch with the failing test in this issue | ||
|
||
If you'd rather open a GitHub issue, here are other ways to share a reproduction (ordered from most helpful to least): | ||
## Option 2: Create a **minimal** reproduction | ||
|
||
- 🥇 Link to a [StackBlitz](https://reactrouter.com/new) environment | ||
- 🥈 Link to a GitHub repository | ||
- 🥉 Description of project including template, config files, `package.json` scripts, etc. | ||
- 🥈 Link to a GitHub repository containing a minimal reproduction app | ||
|
||
## If you are using **Data** or **Declarative Mode**: | ||
|
||
Create a **minimal** reproduction | ||
|
||
- 🥇 Link to a CodeSandbox repro: [TS](https://codesandbox.io/templates/react-vite-ts) | [JS](https://codesandbox.io/templates/react-vite) | ||
- 🥈 Link to a GitHub repository containing a minimal reproduction app | ||
|
||
- type: dropdown | ||
id: mode | ||
attributes: | ||
label: I'm using React Router as a... | ||
description: See https://reactrouter.com/home for explanation | ||
options: | ||
- library | ||
- framework | ||
validations: | ||
required: true | ||
Comment on lines
-41
to
-50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The vast majority of users never seemed to change this anyway so I got rid of it? With a reproduction it'll be easy enough for us to see which mode they're in I think. |
||
- type: textarea | ||
id: reproduction | ||
attributes: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
name: 🚪 Close issues without a reproduction | ||
|
||
on: | ||
workflow_dispatch: | ||
|
||
concurrency: ${{ github.workflow }}-${{ github.ref }} | ||
|
||
jobs: | ||
close-no-repro-issues: | ||
name: 🚪 Close issues without a reproduction | ||
if: github.repository == 'remix-run/react-router' | ||
runs-on: ubuntu-latest | ||
env: | ||
CI: "true" | ||
GH_TOKEN: ${{ github.token }} | ||
steps: | ||
- name: ⬇️ Checkout repo | ||
uses: actions/checkout@v4 | ||
|
||
- name: 📦 Setup pnpm | ||
uses: pnpm/[email protected] | ||
|
||
- name: ⎔ Setup node | ||
uses: actions/setup-node@v4 | ||
with: | ||
# required for --experimental-strip-types | ||
node-version: 22 | ||
cache: "pnpm" | ||
|
||
- name: 📥 Install deps | ||
run: pnpm install --frozen-lockfile | ||
|
||
- name: 🚪 Close Issues | ||
run: node --experimental-strip-types ./scripts/close-no-repro-issues.ts --dryRun |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,11 +52,12 @@ To reduce friction, the SC will primarily operate asynchronously via GitHub, but | |
|
||
Due to the large number of React Router applications out there, we have to be a bit strict on the process for filing issues to avoid an overload in GitHub. | ||
|
||
- **All** bugs must have a **minimal** reproduction [^3] | ||
- Minimal means that it is not just pointing to a deployed site or a branch in your existing application | ||
- **All** bugs must have a **minimal** and **runnable** reproduction [^3] | ||
- _Minimal_ means that it is not just pointing to a deployed site or a branch in your existing application | ||
- _Runnable_ means that it is a working application where we can see the issue, not just a few snippets of code that need to be manually reassembled into a running application | ||
- The preferred methods for reproductions are: | ||
- StackBlitz for **Framework Mode**: https://reactrouter.com/new | ||
- CodeSandbox for **Data/Declarative Modes**: [TypeScript](https://codesandbox.io/templates/react-vite-ts) | [Javascript](https://codesandbox.io/templates/react-vite) | ||
- **Framework Mode**: [StackBlitz](https://reactrouter.com/new) or a GitHub fork with a failing integration test based on `bug_reeport_test.ts` | ||
- **Data/Declarative Modes**: [CodeSandbox (TS)](https://codesandbox.io/templates/react-vite-ts) or [CodeSandbox (JS)](https://codesandbox.io/templates/react-vite) | ||
- If StackBlitz/CodeSandbox is not an option, a GitHub repo based on a fresh `npx create-react-router` app is acceptable | ||
- Only in extraordinary circumstances will code snippets or maximal reproductions be accepted | ||
- Issue Review | ||
|
@@ -139,5 +140,3 @@ To get a feature accepted and implemented in React Router, it will go through th | |
|
||
- A proposal enters **Stage 5 — Stable** once it receives **Stage 4 — Stabilization** PR approvals from at least 50% of the SC members and is merged to `dev` | ||
- This will include the stable feature in `nightly` releases and the next normal SemVer release | ||
|
||
[mermaid]: https://mermaid.live/edit#pako:eNqVkm9r2zAQxr_KcTDYwAn-E8eOGYXGScdeFEYzGLQuQ4nPjsCWjCx3beN89yn2rJKX06tDd8_vnpPuhAeZEyZYVPLP4ciUhp-bTIA5t0_fc2LPMJvd9L8U1wQ_lGxky6oe1k8Z7jQrCdyve3Xzecp8yfB5VK8H3XQPrGmUfDFBoWQNPuxSuKd6T6rtIbUwD2aQStHynBTTXAqLS0fcA_C6qagmobkogbXQiVazfUW_e9hYjm84t1VzZFa_mfTXRpiGilirIXQ_gSyubG0tLjC4NekP2naiKarly8WJtQGNooK_9nBn5QsjN9GeV_z9eqq7_3f1zWLDf9iKDA8drEnVjOfmL08Xeob6aB4qw8SEORWsq3SGmTib0q7JmaZtzrVUmBSsaslB1mm5exMHTLTqaCracFYqVtuqhglMTviKySqYh4toFUfxIo6DyPUdfMPEW0ZzNwjNvb9wYz8Ko7OD71IagjuPo1UQL1dLb7F0PS8IB9zjkBx70mDpflzJYTMdVLIrj7Z_qS4jjtWKhFmUVHZCYxKe_wLxVeMU | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unused |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
To align with our new [Open Governance](https://remix.run/blog/rr-governance) model, we are now requiring that all issues have a [**minimal** and **runnable** reproduction](https://github.com/remix-run/react-router/blob/main/GOVERNANCE.md#bugissue-process). To that end, we're doing some housekeeping in the repo to clean up existing issues that do not have a valid reproduction. This should get us down to a more manageable number of issues and allow us to be more responsive to existing and newly filed issues. | ||
|
||
We're using a Github actions script to identify issues without a reproduction by looking for a [StackBlitz](https://stackblitz.com/), [CodeSandbox](https://codesandbox.io/), or [Github](https://github.com) link in the issue body. This won't be perfect, so if this issue has a reproduction on another platform, please comment back on here and we can re-open the issue. Similarly, if there's a reproduction buried in a comment, please move the link into the description and comment back. Please tag `@brophdawg11` or `@brookslybrand` in your comment so we get a notification as well 🙂. | ||
|
||
If this issue did not have a reproduction but is still valid, or if you wish to start with a fresh issue, please [create a new issue](https://github.com/remix-run/react-router/issues/new?template=bug_report.yml) with a fresh reproduction against v7 and link to this issue in the new description. | ||
|
||
If this a feature request, please open a new [Proposal Discussion](https://github.com/remix-run/react-router/discussions/new?category=proposals) in React Router, and if it gets enough community support it can be considered for implementation. | ||
|
||
If you have any questions you can always reach out on [Discord](https://rmx.as/discord). Thanks again for providing feedback and helping us make React Router even better! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
import { execSync } from "node:child_process"; | ||
import { parseArgs } from "node:util"; | ||
|
||
const sleep = (ms: number) => | ||
new Promise<void>((resolve) => setTimeout(resolve, ms)); | ||
|
||
const { values: args } = parseArgs({ | ||
options: { | ||
dryRun: { | ||
type: "boolean", | ||
default: false, | ||
}, | ||
}, | ||
strict: true, | ||
}); | ||
|
||
run(); | ||
|
||
async function run() { | ||
let issuesCmd = `gh issue list --search "is:issue state:open label:bug" --limit 250 --json number,body`; | ||
console.log(`Executing command: ${issuesCmd}`); | ||
let result = execSync(issuesCmd).toString(); | ||
let issues = JSON.parse(result) as { number: number; body: string }[]; | ||
let noReproIssues = issues.filter(({ body }) => { | ||
return ( | ||
!/https?:\/\/stackblitz\.com\//.test(body) && | ||
!/https?:\/\/codesandbox\.io\//.test(body) && | ||
// Look for github links excluding user-uploaded screenshots | ||
!/https?:\/\/github\.com\/(?!user-attachments)/.test(body) | ||
); | ||
}); | ||
|
||
console.log( | ||
`Found ${noReproIssues.length} issues without a reproduction:\n` + | ||
noReproIssues.map((i) => i.number).join(",") | ||
); | ||
|
||
for (let issue of issues) { | ||
console.log(`--- Processing issue #${issue.number} ---`); | ||
let commentCmd = `gh issue comment ${issue.number} -F ./scripts/close-no-repro-issues.md`; | ||
let commentResult = runCmdIfTokenExists(commentCmd); | ||
console.log(`Commented on issue #${issue.number}: ${commentResult}`); | ||
await sleep(250); | ||
|
||
let closeCmd = `gh issue close ${issue.number} -r "not planned"`; | ||
runCmdIfTokenExists(closeCmd); | ||
// No log here since the GH CLI already logs for issue close | ||
await sleep(250); | ||
} | ||
|
||
console.log("Done!"); | ||
} | ||
|
||
function runCmdIfTokenExists(cmd: string) { | ||
if (args.dryRun) { | ||
console.log(`⚠️ Dry run, skipping command: ${cmd}`); | ||
return "<skipped>"; | ||
} | ||
|
||
if (process.env.CI !== "true") { | ||
console.log(`⚠️ Local run without CI env var, skipping command: ${cmd}`); | ||
return "<skipped>"; | ||
} | ||
|
||
return execSync(cmd).toString(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this to result in an issue being filed instead of a PR. The issue can point to the fork with the failing e2e test, and then we as maintainers can easily check out the fork and run the failing test and work on a fix. But having a PR with a bug report test always felt a bit odd to me:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me! Thanks for reconsidering what had been written for a while at this point