|
| 1 | +# GitHub Copilot – PR Review Instructions for Mendix Web Widgets |
| 2 | + |
| 3 | +Use this guide to review both code and workflow. Focus on Mendix pluggable widget conventions, Atlas UI styling, React best practices, and our release process. |
| 4 | + |
| 5 | +## Repo context |
| 6 | + |
| 7 | +- **Monorepo** with packages under `packages/`: |
| 8 | + - `packages/pluggableWidgets/*-web` |
| 9 | + - `packages/modules/*` |
| 10 | + - `packages/customWidgets/*` |
| 11 | + - `packages/shared/*` (configs, plugins) |
| 12 | +- **Stack**: TypeScript, React, SCSS, Rollup via `@mendix/pluggable-widgets-tools`, Jest/RTL, ESLint/Prettier. |
| 13 | +- **Commands** (root): `pnpm lint`, `pnpm test`, `pnpm build`, `pnpm -w changelog`, `pnpm -w version`. |
| 14 | + |
| 15 | +## What to check on every PR |
| 16 | + |
| 17 | +### PR metadata and process |
| 18 | + |
| 19 | +- **Title format**: |
| 20 | + - If JIRA: `[XX-000]: description` |
| 21 | + - Else: conventional commits (e.g., `feat: ...`, `fix: ...`). |
| 22 | +- **Template adherence** (see `.github/pull_request_template.md`): |
| 23 | + - Lint/test locally: `pnpm lint`, `pnpm test`. |
| 24 | + - New tests for features/bug fixes. |
| 25 | + - Related PRs linked if applicable. |
| 26 | + - If XML or behavior changes: ask for docs PR link in `mendix/docs`. |
| 27 | +- **Multi-package PRs**: Validate each changed package separately. |
| 28 | + |
| 29 | +### Versioning and changelog (per changed package) |
| 30 | + |
| 31 | +- If runtime code, public API, XML schema, or behavior changes in a package: |
| 32 | + - **Require semver bump** in that package's `package.json`. |
| 33 | + - **Require `CHANGELOG.md` update** (Keep a Changelog, semver). |
| 34 | + - Suggest: `pnpm -w changelog` (or update manually). |
| 35 | +- If refactor/docs/tests-only: bump not required (ask author to confirm). |
| 36 | +- Multiple changed packages: each needs its own bump and changelog entry. |
| 37 | + |
| 38 | +## Code quality – Mendix pluggable widgets and React |
| 39 | + |
| 40 | +### Mendix-specific |
| 41 | + |
| 42 | +- **XML ↔ TSX alignment**: lowerCamelCase property keys; TS props/types updated with XML changes; unique widget ID; captions/descriptions/defaults valid. |
| 43 | +- **Mendix data API**: |
| 44 | + - Check `ActionValue.canExecute` before `execute()`. |
| 45 | + - Use `EditableValue.setValue()` for two-way binding. |
| 46 | + - Render loading/empty states until values are ready. |
| 47 | + |
| 48 | +### React code-logic best practices |
| 49 | + |
| 50 | +- **Hooks and effects** |
| 51 | + |
| 52 | + - Correct `useEffect`/`useMemo`/`useCallback` dependencies; avoid stale closures. |
| 53 | + - No side effects in render. Cleanup subscriptions/timers on unmount. |
| 54 | + - Guard async effects to avoid setting state after unmount: |
| 55 | + ```ts |
| 56 | + useEffect(() => { |
| 57 | + let active = true; |
| 58 | + (async () => { |
| 59 | + const data = await load(); |
| 60 | + if (active) setData(data); |
| 61 | + })(); |
| 62 | + return () => { |
| 63 | + active = false; |
| 64 | + }; |
| 65 | + }, [load]); |
| 66 | + ``` |
| 67 | + - Avoid deriving state directly from props unless necessary; prefer computing from props or synchronize carefully (watch for loops). |
| 68 | + |
| 69 | +- **State management** |
| 70 | + |
| 71 | + - Use functional updates when reading previous state: |
| 72 | + ```ts |
| 73 | + setCount(c => c + 1); |
| 74 | + ``` |
| 75 | + - Avoid unnecessary state for values derived from props. Keep state minimal and source-of-truth clear (controlled vs uncontrolled). |
| 76 | + - **MobX stores** for complex cross-component state; **React state** for simple UI state; **Mendix props** as source of truth for persistent data. |
| 77 | + |
| 78 | +- **Rendering and lists** |
| 79 | + |
| 80 | + - Use stable, unique `key`s (avoid array index unless list is static). |
| 81 | + - Avoid heavy computations in render; memoize when there's proven benefit. |
| 82 | + - For large lists/tables, consider virtualization. |
| 83 | + |
| 84 | +- **Performance hygiene** |
| 85 | + |
| 86 | + - Limit `useCallback`/`useMemo` to cases with measurable re-render cost; ensure dependency arrays are correct. |
| 87 | + - Avoid creating new objects/arrays/styles inline when passed to children repeatedly; memoize where needed. |
| 88 | + |
| 89 | +- **Composition and props** |
| 90 | + |
| 91 | + - Prefer composition over prop drilling; consider Context when appropriate. |
| 92 | + - Don't spread unknown props onto DOM nodes (avoid React unknown prop warnings). Validate/filter props before spreading. |
| 93 | + |
| 94 | +- **Accessibility** |
| 95 | + |
| 96 | + - Semantic elements, proper ARIA, focus management, and keyboard navigation. |
| 97 | + |
| 98 | +- **Error handling and robustness** |
| 99 | + |
| 100 | + - Handle null/undefined from Mendix props; safe optional chaining; avoid non-null assertions unless justified. |
| 101 | + - Guard external data parsing; provide graceful fallbacks. |
| 102 | + - Avoid `dangerouslySetInnerHTML`; if unavoidable, sanitize input. |
| 103 | + |
| 104 | +- **MobX best practices** |
| 105 | + - Use `makeAutoObservable(this)` or `makeObservable(this, { ... })` in store constructors. |
| 106 | + - Mark state mutations as `action`; use `computed` for derived values; avoid side effects in computed. |
| 107 | + - Use `reaction()` for side effects, not `autorun()` in most cases. |
| 108 | + - Integrate with React via `useSubscribe()` hook from `@mendix/widget-plugin-mobx-kit`. |
| 109 | + - Keep stores focused and avoid deep nesting; prefer composition over inheritance. |
| 110 | + |
| 111 | +### Styling and theming |
| 112 | + |
| 113 | +- Use **SCSS**; avoid inline styles for static design. |
| 114 | +- Prefer **Atlas UI classes** (e.g., `btn`, `badge`); do not override core Atlas classes. |
| 115 | +- Prefix custom classes with widget name; BEM-like naming. |
| 116 | +- Responsive: relative units/media queries; avoid `!important`. |
| 117 | + |
| 118 | +### Project/tooling consistency |
| 119 | + |
| 120 | +- Follow ESLint/Prettier; do not change existing indentation style. |
| 121 | +- For new deps: justify need/size; prefer tree-shakable imports; respect root `pnpm` versions/overrides. |
| 122 | +- Don't modify generated bundles/patch files unless intended. |
| 123 | + |
| 124 | +## Heuristics Copilot should apply |
| 125 | + |
| 126 | +### Release/workflow checks |
| 127 | + |
| 128 | +- Code or XML changed in `packages/**/<pkg>/` but no version bump or `CHANGELOG.md`: |
| 129 | + - "Behavior changed but no version bump or `CHANGELOG.md`. Please bump semver and add changelog (you can use `pnpm -w changelog`)." |
| 130 | +- Features/bug fixes without tests: |
| 131 | + - "Please add/adjust unit tests in `src/__tests__/` to cover this change." |
| 132 | +- XML changes without TS alignment: |
| 133 | + - "XML props changed but TS props/usage aren't aligned. Please update the component props/types and usage." |
| 134 | + |
| 135 | +### React logic checks |
| 136 | + |
| 137 | +- **Effect dependencies/stale closure**: |
| 138 | + - If an effect references variables not in the dependency array, request adding them or restructuring. |
| 139 | +- **Async effect race**: |
| 140 | + - If an async effect sets state without a guard, suggest guarding or aborting as shown above. |
| 141 | +- **Functional state updates**: |
| 142 | + - If using `setX(x + 1)` with potential stale reads, suggest `setX(x => x + 1)`. |
| 143 | +- **Derived state anti-pattern**: |
| 144 | + - If `useState(props.someValue)` is used to mirror props without sync logic, suggest computing from props or explaining sync strategy. |
| 145 | +- **List keys**: |
| 146 | + - If `index` is used as `key` in dynamic lists, request a stable unique key (e.g., id). |
| 147 | +- **Unnecessary memo/callback**: |
| 148 | + - If `useMemo`/`useCallback` wraps cheap operations or has incorrect deps, suggest removing or fixing deps. |
| 149 | +- **Inline allocations**: |
| 150 | + - Repeated inline objects/arrays/styles passed to children: suggest memoization to reduce renders. |
| 151 | +- **Controlled vs uncontrolled**: |
| 152 | + - Inputs switching between `value` and `defaultValue` or missing `onChange` with `value`: flag and ask to make it consistently controlled or uncontrolled. |
| 153 | +- **Unknown DOM props**: |
| 154 | + - Spreading arbitrary props to DOM nodes: ask to filter out non-standard props. |
| 155 | + |
| 156 | +### MobX logic checks |
| 157 | + |
| 158 | +- **Store setup**: |
| 159 | + - If a class has observable state but no `makeObservable`/`makeAutoObservable`, request adding it. |
| 160 | +- **Action boundaries**: |
| 161 | + - If state is mutated outside `action`, suggest wrapping in `action` or `runInAction`. |
| 162 | +- **Computed purity**: |
| 163 | + - If `computed` properties have side effects or mutations, suggest moving to `reaction` or regular methods. |
| 164 | +- **React integration**: |
| 165 | + - If MobX stores are used without `observer` HOC or `useSubscribe` hook, request proper React integration. |
| 166 | +- **Store architecture**: |
| 167 | + - If stores are deeply nested or overly complex, suggest breaking into focused, composable stores. |
| 168 | + |
| 169 | +### Styling/scroll behavior |
| 170 | + |
| 171 | +- Prefer root-cause layout/size fixes instead of programmatic scroll resets. |
| 172 | + |
| 173 | +## Scope/Noise reduction |
| 174 | + |
| 175 | +- Focus on: `src/**`, `*.xml`, `*.scss`, `package.json`, `CHANGELOG.md`, build/test config changes. |
| 176 | +- Generally ignore: `dist/**`, lockfile-only churn, generated files. |
| 177 | +
|
| 178 | +## Quick commands |
| 179 | +
|
| 180 | +- Lint: `pnpm lint` |
| 181 | +- Test: `pnpm test` |
| 182 | +- Build: `pnpm build` |
| 183 | +- Prepare changelog/version (workspace): `pnpm -w changelog`, `pnpm -w version` |
| 184 | +
|
| 185 | +## Tone and format for comments |
| 186 | +
|
| 187 | +- Be specific and actionable; reference files/lines. |
| 188 | +- Prefer small, concrete suggestions; include short code snippets when helpful. |
0 commit comments