VIDSOL-623: docs: add coding rules to Copilot instructions - hook patterns and American English

[czoli1976]

What and Why

This PR adds three new rules to .github/copilot-instructions.md based on mistakes caught during code review and CI failures in PR #396 (VIDSOL-623).

Rules added

1. American English spelling

The project uses cspell configured for American English. British spellings such as synchronised, initialise, behaviour, or cancelled fail the 👮 QA CI job. This rule makes the constraint explicit so contributors know before they push.

2. No setState directly in effect bodies

The project enforces a non-standard react-hooks/set-state-in-effect ESLint rule that flags any synchronous setState call at the top level of a useEffect/useLayoutEffect callback. The fix is to wrap the call inside a named helper function declared within the effect. This rule documents the pattern with a before/after example.

3. No setState during render (derived-state pattern)

Calling setState inside the render body when a prop differs from a tracked state value can cause infinite render loops if the caller passes a non-stable reference (e.g. an object literal). The correct approach is to use a useRef to track the last-seen prop and derive the return value directly during render — ref mutation is safe during render and does not trigger a re-render. This rule documents the anti-pattern and the correct ref-derived-return implementation.

---

Reviewers: please pay particular attention to whether the code examples are clear and actionable. The goal is for Copilot (and human contributors) to catch these patterns before code review.

[Copilot]

Pull request overview

This PR updates .github/copilot-instructions.md with three new rules to help contributors (and Copilot) avoid mistakes that caused CI failures in PR #396 (the Video Stats Overlay feature). The rules cover American English spelling enforcement, preventing direct setState calls in effect bodies, and preventing setState during render.

Changes:

• Adds a ## Spelling subsection documenting the @cspell/eslint-plugin American English enforcement and common trap words.
• Adds a ## setState in effects and during render section documenting the rule against direct setState in useEffect/useLayoutEffect bodies, with a named-inner-function workaround.
• Adds documentation for the derived-state-during-render anti-pattern and the correct useRef-based approach for synchronously deriving values from a changed prop.

---

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

The inline code comment on this line repeats the incorrect rule name react-hooks/set-state-in-effect. It should be updated to react-hooks/no-direct-set-state-in-use-effect to be consistent with the actual rule name in eslint-plugin-react-hooks v7.

Suggested change

	- **Rule:** Never call `setState` directly inside an effect body (`useEffect` or `useLayoutEffect`). The project enforces `react-hooks/set-state-in-effect`, which flags any synchronous `setState` call at the top level of an effect callback.
	- **Rule:** When you need to update state from inside an effect, wrap the call in a **named helper function** declared inside the effect. Calls through a function are not flagged by the rule.
	**Violation:**
	```tsx
	// Bad: setState called directly in effect body — triggers react-hooks/set-state-in-effect
	- **Rule:** Never call `setState` directly inside an effect body (`useEffect` or `useLayoutEffect`). The project enforces `react-hooks/no-direct-set-state-in-use-effect`, which flags any synchronous `setState` call at the top level of an effect callback.
	- **Rule:** When you need to update state from inside an effect, wrap the call in a **named helper function** declared inside the effect. Calls through a function are not flagged by the rule.
	**Violation:**
	```tsx
	// Bad: setState called directly in effect body — triggers react-hooks/no-direct-set-state-in-use-effect

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

---

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

---

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

The closing triple-backtick for the preceding **Correct (explicit linear flow):** code block (which opened at line 1634) was removed in this diff. The new --- horizontal rule is placed immediately after }; on line 1681 without closing the code fence first. This breaks the Markdown structure of the document — the entire section from --- through the ## setState in effects and during render content will be rendered as part of the previous code block, making the new rules invisible and unformatted.

The closing ``` must be re-added before the --- separator.

[johnny-quesada-developer]

Hi @czoli1976, it looks like the code example got cut off here.

[johnny-quesada-developer]

I'd prefer to rely on the plugin doing its job rather than unnecessarily increasing the docs. Don't you think this is redundant, @czoli1976? Also, if these instructions are specifically for AI models, I'd argue that it is unlikely for them to have to many grammar mistakes...

[johnny-quesada-developer]

Hi @czoli1976, it looks like the code example got cut off here.

[johnny-quesada-developer]

Hi @czoli1976,

I’m not sure these rules are beneficial.

They implicitly encourage relying on useEffect as a side-effect listener, which tends to push code toward an observable-style pattern. In practice, this often makes components harder to follow, maintain, and debug.

Whenever possible, state transitions should happen in a linear flow during render or in event handlers, rather than being triggered indirectly through effects.

In general, effects should be used to hook into the component lifecycle or synchronize with external systems, not as listeners or triggers for internal state changes.

We already have some rules addressing this area, and we also have the derived-state-during-render rule enabled. Because of that, adding this rule seems both redundant and potentially misleading.

Given this, I think it would be better not to introduce these rules:

• Rule: Never call setState directly inside an effect body (useEffect or useLayoutEffect). To follow React best practices, enable the ESLint rule react-hooks/no-direct-set-state-in-use-effect, which flags any synchronous setState call at the top level of an effect callback.
  • Already activated, redundant
• Rule: When you need to update state from inside an effect, wrap the call in a named helper function declared inside the effect instead of calling setState inline.
  • We should discourage use of state on useEffects unless is strictly necessary
• Rule: Never call setState during the render function body (derived-state-during-render pattern). This can cause render loops when a caller passes a non-stable reference, and React may log warnings.
  • Linter already warms about this
• Rule: When you need to derive fresh values from a changed prop without waiting for an effect, track the previous prop value in a useRef and derive the return value directly during render — mutating a ref does not trigger a re-render.
  • Example incomplete, could encourage pour UX, there is no way of knowing if stats are been requested, or publisher was not provided, also looks like a very specific use case not so valuable.

[johnny-quesada-developer]

This is misleading... you should run the test for all the projects, we have a mono repo which declare varios projects which then are reuse in others, so green flags at project level doesn't mean that other projects where not affected.

[OscarFava]

We can delete it by now.