$npx -y skills add omnifaces/claude-faces-expert --skill faces-reviewReview Jakarta Faces code for common mistakes, anti-patterns, and rule violations
| 1 | *Version 1.3.0* |
| 2 | |
| 3 | Review the Jakarta Faces code in `$ARGUMENTS` (if no argument, scan the project for `.xhtml` and backing bean files) against the rules in `.claude/faces/rules.md` and its topic files. |
| 4 | |
| 5 | ## Step 1: Determine the Faces Version |
| 6 | |
| 7 | Before reviewing anything, establish BOTH Faces versions — they are not per definition the same. See "Version Discipline" in `.claude/faces/rules.md`. |
| 8 | |
| 9 | **Runtime version** (which APIs exist), in this order: |
| 10 | |
| 11 | 1. An explicit `jakarta.faces-api`/`javax.faces` dependency, or a bundled Faces implementation (`org.glassfish:jakarta.faces` for Mojarra up to 4.x, `org.glassfish.mojarra:mojarra` for Mojarra 5.0+, `org.apache.myfaces.core:myfaces-impl` for MyFaces) — whose major.minor tracks the Faces version it implements. |
| 12 | 2. Otherwise the `jakarta.jakartaee-api`/`jakarta.jakartaee-web-api` version, mapped via the version lineage in `.claude/faces/rules.md` (full Jakarta EE server only). |
| 13 | 3. Otherwise the target application server, or as a last resort the XML namespaces used in the `.xhtml` files. |
| 14 | |
| 15 | **Declared version** (which descriptor schema applies): the `version` attribute and `xsi:schemaLocation` of `faces-config.xml` — see the Configuration checklist below. |
| 16 | |
| 17 | State both at the top of the report, with how each was detected. If the runtime version cannot be determined, say so and assume the most recent RELEASED version (Faces 4.1) — never assume an unreleased one. Never substitute the declared version for the runtime version. |
| 18 | |
| 19 | Then hold every suggestion to the runtime version: |
| 20 | |
| 21 | - Verify an API before recommending it, against the sources listed under "References" in `.claude/faces/rules.md`, using the set for the detected runtime version — this skill has no web access, so delegate the lookup to an `Agent`. If it cannot be verified, do not recommend it; report the API as unconfirmed instead. |
| 22 | - A genuinely useful newer API may be mentioned only as `info` severity, explicitly labelled with the version that introduces it and the fact that it requires an upgrade. It is never an `error` or `warning`. |
| 23 | |
| 24 | Finally, establish the project's MARKUP STYLE the same way — plain Faces components (`<h:inputText>`) or passthrough elements (`<input type="text" faces:value="...">`) — by taking the dominant one across the `.xhtml` files. Both are valid; see "Passthrough Elements" in `.claude/faces/rules.md`. When there is no dominant style, as in a new or empty project, default to plain Faces components. |
| 25 | |
| 26 | - Write every suggestion in the detected style. Do not nudge a project toward the other one. |
| 27 | - Report a view that mixes both as `info`, never as `error` or `warning`. |
| 28 | - NEVER propose a wholesale conversion from one style to the other, and never as a "modernisation". Some components have no passthrough element form at all, so such a conversion can only ever be partial, which is worse than either style consistently applied. |
| 29 | |
| 30 | ## Step 2: Review Checklist |
| 31 | |
| 32 | For every non-trivial construct, ask in this order: (1) is it still needed, or does a built-in already do this? (2) is the mechanism available in the detected runtime version? (3) is it implemented correctly? Report at the highest level that fires and stop — NEVER propose modernising something you would delete. |
| 33 | |
| 34 | ### Superseded Constructs |
| 35 | |
| 36 | Hand-rolled code that a built-in has since replaced. Report `info` when it merely duplicates the built-in, `warning` when the hand-rolled version has a defect the built-in does not. The replacement MUST exist in the detected runtime version. Suggest a plain Faces replacement; only propose an OmniFaces one when the project already depends on OmniFaces. |
| 37 | |
| 38 | | Hand-rolled | Built-in | Since | |
| 39 | |---|---|---| |
| 40 | | `PhaseListener` shuttling `FacesMessage`s through the session to survive a redirect | navigation from `<f:viewAction>`, which keeps them automatically; else `Flash.setKeepMessages(true)` at the redirecting call site | 2.2 | |
| 41 | | `immediate="true"` on an ajax `UICommand` to skip input processing | `<f:ajax>` — `execute` already defaults to `@this` | 2.0 | |
| 42 | | `isPostback()` guard in `@PostConstruct` for GET-only initialization | `<f:viewAction>` | 2.2 | |
| 43 | | Custom converter for a standard type | implicit converters (incl. `UUID` since 4.1) | varies | |
| 44 | | `Application.evaluateExpressionGet()` calls | `@Inject @ManagedProperty` | 2.3 | |
| 45 | | Hand-rolled server push (one-way, server to client) | `<f:websocket>`/`<o:socket>`, or `<o:sse>` | Faces 2.3 / OmniFaces 5.2 | |
| 46 | |
| 47 | A `PhaseListener` calling `Flash.setKeepMessages(true)` on every postback is not a fix for the first row: the flash saves the messages whether or not the response is a redirect, so they are rendered again on the next request and keep echoing forward. A `NavigationHandler` decorator checking `NavigationCase.isRedirect()` |