diff --git a/.agents/skills/how-to-write-component/SKILL.md b/.agents/skills/how-to-write-component/SKILL.md index f22b1d3727f..943e8e56650 100644 --- a/.agents/skills/how-to-write-component/SKILL.md +++ b/.agents/skills/how-to-write-component/SKILL.md @@ -12,13 +12,15 @@ Use this skill to make component architecture explicit before implementation. Cl Then mark instance isolation/reset as a lifecycle requirement, not another node type. Do not force one label to answer both axes. Read only the bundled references required by the current change. +Make shared feature state visible as a dependency graph instead of hiding it in a component, a giant hook, or a provider adapter. When a value drives a query or command, feeds reusable derivation, is consumed by another owner, or bridges an external source for several consumers, it has entered the feature state graph. In this repository, use feature-local Jotai as the default representation for that graph. Keep owner-local display state and submit-only fields in the component or DOM. + ## Operating Modes Choose the mode from the user's requested outcome: - **Architecture audit:** Perform a read-only analysis of an existing component. Trace the complete locally owned rendered tree from the named root to its leaves, inventory every state source and props edge, evaluate ownership and lifecycle, then propose the target state graph and component boundaries. Do not lead with bug findings or severity levels. - **Refactoring design:** Perform the architecture audit, then produce target contracts, migration slices, and a verification strategy without modifying code. -- **Implementation:** Perform enough of the architecture audit to establish ownership, then implement one coherent vertical slice and verify it. +- **Implementation:** Establish the affected target state graph, owner map, and props edges before the first edit, then implement one coherent vertical slice at a time. After the final requested slice, rerun the complete root-to-leaf ownership and props audit before declaring the refactor complete. - **Bug or regression review:** Use the frontend code-review workflow unless the user explicitly requests this skill's architecture model as an additional lens. ## First Decisions @@ -28,12 +30,31 @@ Choose the mode from the user's requested outcome: | Where should code live? | In the product workflow, route, or feature owner. | Several verticals need the same stable contract. | | Who owns state and handlers? | The lowest owner that consumes them and whose lifetime matches the state. | Another owner coordinates the value or it must survive the local owner's unmount. | | Should React control a value? | Leave submit-only DOM fields uncontrolled. | The workflow must own the current value to drive rendering or coordination. | -| Should state enter Jotai? | Keep isolated display state and submit-only fields local. | A value drives a query or command, feeds reusable derivation, is consumed by another owner, or needs scoped workflow persistence. | +| Should state enter Jotai? | Keep isolated display state and submit-only fields local. Once a value enters the feature state graph, use feature-local Jotai first. | An existing stable graph/store already owns the same contract, or the value remains entirely owner-local. | | Who owns URL state? | Next.js route APIs and `nuqs`. | Atoms need a route-identity bridge for queries or shared derivations. URL writes still stay with the URL owner. | | Who owns remote state? | TanStack Query at the lowest consumer. | Atom state drives the query, shared derivations consume its result, or a workflow command coordinates it. | | Is a wrapper needed? | Use the primitive or direct code. | The wrapper owns behavior, validation, state, or semantics. | | Is an Effect needed? | Derive during render or handle the user action. | A named external system must be synchronized. | +## State Graph Entry Gate + +A value enters the feature state graph when any of these are true: + +- it drives a query, mutation, workflow command, or another graph node; +- several sibling owners consume it or a parent would otherwise relay it through more than one layer; +- it is route, URL, Query, or parent-owned input needed by several queries, facts, or commands; +- it needs named derivation, coordinated writes, workflow persistence, or instance-scoped reset. + +For values that enter the graph: + +- create or extend a feature-local state file/folder ordered as inputs -> query/mutation nodes -> field selectors -> named business facts -> commands -> runtime orchestration; +- bridge external owners such as route APIs, `nuqs`, and TanStack Query without copying their state into a second owner; +- expose field selectors, named facts, and domain commands to components, not a complete query result, a hook result object, raw query keys, or `refetch` plumbing; +- keep query and mutation atoms unscoped; scope only primitives and injected snapshots whose instances must reset or remain isolated; +- do not use Context, a provider value object, or a large custom hook as an atom substitute when it merely repackages the same state machine. + +Context remains appropriate for stable external dependencies or an existing authoritative product boundary. Local `useState`, `useQuery`, and `useMutation` remain appropriate when one component owns their complete lifecycle and no other graph node consumes them. + ## Topic Routing - Architecture audits, component moves, module boundaries, props, types, owner placement, or parent input that creates a child state graph: read [`references/ownership.md`][ownership]. @@ -47,13 +68,28 @@ Choose the mode from the user's requested outcome: Follow this order so component splitting does not precede ownership decisions: -1. **Boundary:** identify the route, tab, workflow, or action surface that owns the behavior and state lifetime. +1. **Boundary:** identify the route, tab, workflow, or action surface that owns the behavior and state lifetime. Inspect nearby sibling feature state files and established graph boundaries before inventing a new pattern. 2. **Data contract:** identify generated API types, URL inputs, Query cache data, and user-input normalization boundaries. -3. **State graph:** list graph inputs, query/mutation nodes, named derived facts, commands, and any scope/reset needs. Keep unrelated local UI state out. +3. **State graph:** list graph inputs, query/mutation nodes, field selectors, named derived facts, commands, runtime controllers, and scope/reset needs. Decide explicitly for every stateful value whether it enters the graph. When a graph exists, create its feature-local state file before wiring components. Keep unrelated local UI state out. 4. **Re-cut the component tree:** treat current components and files as evidence, not target constraints. For every current local component, decide whether to keep, split, merge, remove, rename, promote to an owner, or demote to presentation based on state lifetime, behavior ownership, interaction lifecycle, and independently changing visual regions. 5. **Component contracts:** place data, loading, empty, error, and handlers at the lowest real consumer; define only the props that cross true owner boundaries. 6. **Interaction surfaces:** give forms, menus, dialogs, drawers, and popovers explicit lifecycle owners. -7. **Finish and verify:** remove copied state, unnecessary Effects/wrappers/memoization/nullable coercion, then verify observable behavior at the narrowest sufficient boundary. +7. **Finish and verify:** remove copied state, unnecessary Effects/wrappers/memoization/nullable coercion, rerun the props-edge and owner ledger, then verify observable behavior at the narrowest sufficient boundary. + +## Implementation Completion Gate + +Do not describe an implementation or refactor as complete until all applicable checks pass: + +- every multi-consumer graph value has one authoritative bridge or graph node and a named owner; +- route/URL identity is read directly by a single consumer or bridged once; the same identity is not also threaded through descendant props; +- query keys, observer objects, `refetch`, loading/error groups, and cache invalidation mechanics do not cross a component boundary unless the child is the actual query surface owner; +- no page, hook, provider, or Context mainly destructures, renames, and redistributes another state machine; +- components consume field selectors, named facts, or domain commands instead of rebuilding shared business conclusions; +- query/mutation atoms remain cache-shared, while only resettable workflow primitives or boundary snapshots are scoped; +- every remaining multi-layer prop edge is listed and justified as stable identity, an immutable display snapshot, placement, or a named cross-boundary command; +- the final root-to-leaf rendered tree, state graph, owner map, and reset boundaries match the implemented code rather than only the initial design. + +If the user requests incremental commits, a slice may intentionally leave documented edges for a later slice. The final slice must still pass this gate. ## Architecture Audit Output @@ -61,13 +97,13 @@ In architecture-audit mode, report: 1. **Review boundary:** the root component, locally owned rendered paths, and stopping boundaries. 2. **Rendered component tree:** every local branch and secondary surface followed during the audit. -3. **State inventory:** owner/source, graph role, declaration, consumers, and lifetime/reset needs for every stateful value, query, mutation, ref, custom-hook result, derived fact, and command. +3. **State inventory:** owner/source, graph role, graph-entry decision, declaration, consumers, and lifetime/reset needs for every stateful value, query, mutation, ref, custom-hook result, derived fact, and command. 4. **Props-edge ledger:** every meaningful parent-child props edge and whether each prop is consumed, forwarded, renamed, recomputed, mirrored, or paired with lifecycle state. 5. **Current state graph:** primitive inputs -> queries/mutations -> named facts -> commands -> consumers. 6. **Ownership assessment:** misplaced state, switchboard parents, duplicated owners, mirrored state, prop fan-out, and unclear lifecycle boundaries. 7. **Component-boundary disposition:** account for every current local component as keep, split, merge, remove, rename, promote to owner, or demote to presentation. Map every current state/workflow owner to a target component, and justify boundary changes by ownership, lifecycle, behavior, or an independently changing visual region. 8. **Target architecture:** redraw the target rendered component tree independently of the current file layout, then report proposed owners, component contracts, reset boundaries, and the target state graph. Do not count a renamed component, facade wrapper, props bag, or provider around the same switchboard as a boundary redesign. -9. **Migration slices:** ordered refactoring steps with explicit component moves/splits/merges and observable verification boundaries. +9. **Migration slices:** ordered refactoring steps with explicit state-file changes, component moves/splits/merges, observable verification boundaries, and intentionally temporary props edges. Do not organize an architecture audit by bug severity unless the user also requests a correctness review. Use compact tables where they make full accounting easier to verify: @@ -86,13 +122,16 @@ Do not organize an architecture audit by bug severity unless the user also reque ## Patterns To Avoid - A giant component, switchboard page, or view-model hook that redistributes a large props-and-handlers bag: move single-branch state down and expose focused feature facts and commands for shared workflows. +- A provider or Context that converts a giant hook or props bag into several value objects without changing the underlying owner graph. +- Route identity threaded through the component tree after it has already been bridged into a feature graph or is available from an authoritative route/product context. +- Query keys, observer methods, cache invalidation details, or `data/pending/error/retry` groups passed from a parent that does not render or coordinate those states. - A second owner created by copying props, URL values, or Query data into local state or atoms: bridge the authoritative owner instead. - Components that consume a whole query atom or repeatedly derive the same business conclusion: expose a field selector or named fact. - Query or mutation atoms placed in a scope, or an edit-session snapshot overwritten by forced hydration: scope only the primitives whose instances must reset. - Effects used to fetch data, copy render state, react to user actions, or reset from props: derive during render, handle the event, or use the owning data API. - Wrappers, helpers, memoization, or nullable coercion that only hide unclear ownership: fix the boundary before adding abstraction or optimization. -Read the nearby implementation and tests before analyzing or changing code. In architecture-audit and refactoring-design modes, do not modify code. When implementation is requested, implement one coherent vertical slice; do not expand into equivalent patterns elsewhere unless the current contract cannot be completed without them. Run the checks documented by the owning package: `web/docs/test.md` or `web/docs/lint.md` for Web, and `packages/dify-ui/docs/testing.md` for Dify UI. +Read the nearby implementation, tests, sibling state files, and existing feature boundaries before analyzing or changing code. In architecture-audit and refactoring-design modes, do not modify code. When implementation is requested, implement one coherent vertical slice; complete equivalent ownership fixes inside the audited feature when the target contract depends on them, but do not expand into unrelated features. Run the checks documented by the owning package: `web/docs/test.md` or `web/docs/lint.md` for Web, and `packages/dify-ui/docs/testing.md` for Dify UI. [data]: references/data.md [interactions]: references/interactions.md diff --git a/.agents/skills/how-to-write-component/references/data.md b/.agents/skills/how-to-write-component/references/data.md index 9c9b50bf7a6..5a02fed5fc2 100644 --- a/.agents/skills/how-to-write-component/references/data.md +++ b/.agents/skills/how-to-write-component/references/data.md @@ -17,10 +17,12 @@ Read this document when a component consumes generated contracts, nullable API v - Use generated options directly with `useQuery(consoleQuery.xxx.queryOptions(...))`, `marketplaceQuery`, or the equivalent generated client. - If query input comes from atom state, keep it in `atomWithQuery`; do not unwrap the atom in a component solely to call `useQuery`. - Keep owner-local queries in `useQuery` when their pending, error, and data state serve only that component. Promote them to query atoms only when atom input drives them or other graph nodes consume their result. +- When a query supplies several sibling owners, shared derived facts, or workflow commands, model it as a graph node with field selectors. Do not keep it in a page and pass its data, query key, observer methods, and status fields back down as a deconstructed query object. - For missing required input, branch the whole generated input with `skipToken`. Add `enabled` only for an independent execution condition; do not put `skipToken` inside a placeholder payload or coerce IDs to empty strings. - Return generated `queryOptions()`, `infiniteOptions()`, or `mutationOptions()` directly from TanStack Query atoms. Pass supported options into the generated call instead of spreading into a parallel object. - Share the exact options between prefetch and render when they represent the same request. Do not extract option helpers merely to reuse input construction. - In Jotai-backed components, consume field-level selectors or named facts rather than the complete query result unless observer methods such as `refetch` or an infinite-scroll field group are part of that owner's contract. +- Keep observer methods inside the component that owns the query surface or behind a named graph command. A parent must not pass raw `refetch`, query keys, or invalidation details merely because a descendant action needs fresh data. - Avoid pass-through service hooks that only rename generated options. Keep feature hooks for actual orchestration or shared domain behavior. ## Mutations And Cache @@ -28,6 +30,7 @@ Read this document when a component consumes generated contracts, nullable API v - Use generated `mutationOptions()` directly for owner-local mutations. - Keep a mutation in `useMutation` when pending and error state belong to one dialog or form. Use a mutation atom only when shared workflow orchestration or graph-derived state consumes it; query and mutation atoms remain unscoped. - Put shared invalidation, retries, and cache behavior in `createTanstackQueryUtils(...experimental_defaults...)`. Local callbacks may own toast, close, and navigation effects but must not replace shared cache policy. +- Workflow-specific invalidation belongs beside the mutation/command that understands which caches become stale. Construct generated query options or keys from graph identity there instead of receiving keys from a rendering parent. - Prefer `mutate(...)`. Use `mutateAsync(...)` only when Promise composition is required, and catch awaited failures. - Preserve intentional empty values and current list/detail ownership when updating data. Do not add optimistic updates without a verified owner contract. diff --git a/.agents/skills/how-to-write-component/references/ownership.md b/.agents/skills/how-to-write-component/references/ownership.md index 17911e45c90..dc68ab5fad7 100644 --- a/.agents/skills/how-to-write-component/references/ownership.md +++ b/.agents/skills/how-to-write-component/references/ownership.md @@ -17,6 +17,8 @@ For a named component, use it as the audit root. For a refactoring request, use The audit is incomplete until every locally owned rendered path, meaningful props edge, and stateful value is represented in the component tree, state inventory, or props-edge ledger; every current local component has a target disposition; and every current state/workflow owner maps to a target component and reset boundary. Correctness bugs may illustrate an ownership defect, but architecture-audit mode does not prioritize or severity-rank bug findings. +For implementation work, repeat this audit against the final code. A target diagram written before implementation is not evidence that ownership changed. Record remaining route identities, query mechanics, refs, snapshots, and commands that cross each boundary; unexplained leftovers mean the refactor is incomplete. + ## Vertical Modules - Organize code by product workflow, route, or behavior owner. Keep components, hooks, local types, atoms, query helpers, tests, and small utilities beside the code that changes with them. @@ -29,7 +31,8 @@ The audit is incomplete until every locally owned rendered path, meaningful prop - Before declaring state, a query, or a workflow hook in a page or parent, identify the direct descendant branches that consume each returned value. - If only one branch consumes a value, declare it in the lowest owner in that branch. The parent may pass stable identity or the smallest boundary input, but must not own child state merely to construct props. -- A parent may own a value when several sibling branches require one live snapshot and the parent genuinely derives or coordinates submission, selection, navigation, lifecycle, loading, or errors. If it only destructures, renames, and forwards fields, it is a switchboard rather than an owner; put the shared workflow in a feature-local state graph or provider so each surface consumes only its named facts and commands. +- A parent may own a value when several sibling branches require one live snapshot and the parent genuinely derives or coordinates submission, selection, navigation, lifecycle, loading, or errors. If it only destructures, renames, and forwards fields, it is a switchboard rather than an owner; put the shared workflow in a feature-local state graph so each surface consumes only its named facts and commands. Use a provider only when it becomes the authoritative input, scope, or external-dependency owner. +- A provider is a real boundary only when it establishes authoritative input, scope/isolation, or a stable external dependency. A provider that calls a large hook and repackages its return value into Context remains a switchboard. - A page or feature root may wire route identity, providers, layout, navigation, and genuine cross-surface coordination. It must not call a child-specific state or query hook merely to assemble that child's props. - Keep child contracts at the ownership boundary: stable domain identity, a small immutable snapshot, placement options, or named cross-boundary commands. Do not pass an internal state machine as separate `data`, `pending`, `error`, `retry`, `open`, setter, and callback props when the parent does not use them, and do not hide the same fan-out in a props bag or hook result object. - Repeated TanStack Query calls in siblings are acceptable when each sibling independently consumes the data; the cache already deduplicates requests. @@ -38,6 +41,8 @@ The audit is incomplete until every locally owned rendered path, meaningful prop - If the child builds queries, dialogs, mutations, derivations, commands, or a reset lifecycle around a stable identity or snapshot, give that boundary a feature-local state file. This does not by itself require a new module or directory. - Pass stable domain identity or the smallest sufficient action snapshot across boundaries. Do not copy props into atoms merely to avoid passing them, and do not pass raw server data together with separately derived flags for the same concept. - One pass-through layer is acceptable for stable identity and placement. It is not permission to relay workflow state and handlers through an unrelated component. +- Route identity may pass once from a framework route into its feature boundary. If multiple descendants, queries, facts, or commands need it, bridge it into the feature graph and stop passing it as props. +- A query snapshot may cross once as immutable display input. Query keys, observer methods, retry/loading/error groups, and invalidation mechanics belong to the query surface or feature graph and must not be decomposed into props. - Keep source selection, defaults, validation, dirty checks, and payload shaping beside the workflow that owns submission. ## Boundaries diff --git a/.agents/skills/how-to-write-component/references/runtime.md b/.agents/skills/how-to-write-component/references/runtime.md index b336fe39033..5069c122589 100644 --- a/.agents/skills/how-to-write-component/references/runtime.md +++ b/.agents/skills/how-to-write-component/references/runtime.md @@ -8,6 +8,7 @@ Read this document when a change introduces Effects, navigation side effects, me - Use Effects only to synchronize with a named external system such as a browser API, subscription, timer, analytics integration, non-React widget, or imperative DOM API. - Do not use Effects to transform render state, handle user actions, copy query data, reset state from props, or fetch data owned by framework APIs or TanStack Query. - Initialize query-backed form sessions after their defaults are available instead of copying data through Effects. Use a stable semantic identity key when the represented identity changes; use the intended surface lifecycle for per-session reset. +- When external synchronization participates in a feature state graph, prefer a small headless runtime controller that reads and writes focused atoms. Do not hide storage, polling, invalidation, and workflow refs in a giant hook that returns a state-and-handler object for another component or provider to redistribute. ## Navigation diff --git a/.agents/skills/how-to-write-component/references/state.md b/.agents/skills/how-to-write-component/references/state.md index d58c404619b..f34f117f27e 100644 --- a/.agents/skills/how-to-write-component/references/state.md +++ b/.agents/skills/how-to-write-component/references/state.md @@ -8,6 +8,8 @@ First identify the owner or source of truth: one component, a parent boundary, U A value should enter a feature state graph when it drives a query or command, feeds a reusable derivation, is consumed by another owner, or bridges an external source for several consumers. Entering the graph does not transfer ownership from URL APIs or Query cache to Jotai. +Once a value enters the graph, represent it in a feature-local Jotai state file by default. Do not leave graph inputs and async nodes inside a component or large custom hook while exporting only the final result through props or Context; that hides the dependency graph instead of modeling it. + Keep values out of the graph when they only affect one component's presentation, are read only at form submission, or are one-off render computations without domain meaning. ## Choose The Owner @@ -16,6 +18,7 @@ Keep values out of the graph when they only affect one component's presentation, - Use feature-scoped Jotai when siblings need one source of truth, values drive other atoms, a parent input establishes its own state graph, or a scoped workflow must preserve state across hidden or unmounted steps. - Keep server and cache state in TanStack Query. Use existing feature stores for complex, high-frequency interaction state such as workflow canvas drag, resize, and runtime panels. - Use feature-owned storage only for low-frequency client preferences, dismissed notices, and UI defaults. Live application state does not belong in local storage. +- Use Context for an existing authoritative product boundary or stable dependency injection. Do not introduce Context merely to translate a hook result, query observer, or prop fan-out into another large value object. ## Forms And Sessions @@ -31,6 +34,7 @@ Keep values out of the graph when they only affect one component's presentation, - A single consumer should read the owner hook directly. Hydrate an unscoped primitive atom at the route or surface boundary only when several consumers, query atoms, or shared derived atoms require route identity. Keep URL writes in route and query-state APIs. - A route bridge that must follow later URL changes needs explicit re-hydration behavior, commonly `dangerouslyForceHydrate: true` at that controlled boundary. A scoped workflow input is different: initialize it once, key the scope by semantic identity when switching entities should reset it, and do not force later parent refreshes into an in-progress session. - Within one route-owned feature, choose one route-identity source. Do not hydrate route identity into atoms while also threading the same ID through multiple component layers. +- A route parameter may cross the route-to-feature entry edge once. After a route bridge exists, queries, facts, commands, and descendant surfaces must read that bridge instead of accepting the same ID as props. - Put shareable filters, tabs, pagination, and search state in the URL. Keep one-shot navigation signals and transient UI state out of persistent subscriptions. ## Build A Clean Jotai Graph @@ -42,6 +46,8 @@ Keep values out of the graph when they only affect one component's presentation, - Use `atomWithQuery` or `atomWithMutation` for async work driven by atom state. Do not hand-roll loading, error, or in-flight state for atom-orchestrated work. - Use `selectAtom` or another field-specific derived atom for query results. `jotai-tanstack-query` does not provide TanStack Query tracked properties, so reading a whole query atom subscribes to the entire observer result. Read the whole result only when the consumer truly needs observer methods or a coordinated group of fields. - Name derived atoms as business facts and write atoms as user or workflow commands. Commands should express actions such as selecting a source or submitting a wizard, not merely rename setter callbacks. +- Treat query keys, observer methods, cache invalidation, retries, and refresh composition as graph internals. Export a named command such as `refreshDocument`, not the query key plus a raw `refetch` callback. +- A headless runtime controller may synchronize atoms with storage, timers, subscriptions, or other external systems. It should read and write focused graph nodes and render no UI; it must not return a large object for a parent to redistribute. ## Scope, Isolation, And Reset @@ -51,6 +57,18 @@ Keep values out of the graph when they only affect one component's presentation, - Scope exists for per-instance isolation and natural reset, not as a general module boundary. A state file may be warranted even when its atoms remain unscoped. - Keep independent dialog lifecycles separate. A scoped open-state atom is acceptable only when composed sibling surfaces would otherwise pass confusing lifecycle props through unrelated owners. +## Graph Review Checklist + +Before implementation and again after the final slice, draw the actual graph in dependency order and verify: + +- every route, URL, parent, and persisted input has one bridge; +- every query driven by graph input is represented in the graph and remains unscoped; +- components read field selectors or named facts rather than complete query results; +- write atoms and exported commands describe user or workflow intent; +- scoped atoms are limited to primitives and snapshots with a documented reset boundary; +- no equivalent value remains available through both atoms and descendant props; +- every exported atom has a real component, boundary, or state-module consumer. + ## Persistence - Use feature-owned storage modules built on `createLocalStorageState`; callers should not scatter direct storage access or raw keys.