ref: convert SentryAppExternalForm to FC#113885
Closed
priscilawebdev wants to merge 8 commits into
Closed
Conversation
Convert the class component to a functional component using hooks, and switch from a default export wrapped in withApi to a named export using useApi internally. Update the two importers to use the named export. Behavior is preserved: state reset is still triggered only on action changes (mirroring the original componentDidUpdate), the FormModel instance persists via useRef, and the debounced option loader is kept stable across renders via useMemo.
github-actions
Bot
added
the
Scope: Frontend
Drop the useCallback for onSubmitError since it's only passed to Form and has no stability requirement. Hoist getElementText out of the component as a module-level function with early returns — it doesn't close over any state, so it has no business being a hook.
Helpers that are only called from renderField (getDefaultOptions, getOptions, createPreserveOptionFunction) are not deps of any hook, so useCallback was paying its own overhead for no benefit. Convert them to plain arrow consts. Also drop the rest-spread pass-through wrapping onSubmitSuccess and return null instead of '' from the empty-installation guard.
getDefaultOptions only closes over resetValues and is a pure mapping from (field, settings) to options — hoist it to module scope so it stops capturing props. createPreserveOptionFunction had exactly one call site, so inline the handler directly into the onChangeOption prop.
Cover the component's own contract: empty-uuid render guard, field rendering from config, successful issue-link POST, alert-rule-action submit shape (settings array + hasSchemaFormConfig), validation gate before alert-rule-action submit, depends_on load gating, reset-value label preservation, and reset-on-action-prop-change.
Replace the weak waitFor(not.toHaveBeenCalled()) / waitFor(toHaveBeenCalled()) pair with a synchronous pre-assertion and an exact count check after typing. Locks in the current per-keystroke reload behavior (6 chars = 6 external calls) so any future debouncing or batching is an explicit, reviewable change.
Every other test in this spec interacts with config fields (type into them, click them, read their values), so a broken render loop would already fail them with clearer errors. The consumer spec in sentryAppRuleModal.spec.tsx also covers the same smoke path.
Annotating baseProps with ComponentProps<typeof SentryAppExternalForm> narrows the literal strings against the Props type, so the `as const` sprinkles on `action` and `element` become unnecessary.
Member
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9924ddc. Configure here.
Member
Author
|
Closing — the FC conversion didn't end up simplifying the migration review. Focusing on #112911 instead. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Converting to FC here, so it will be easier to review the migration to the BackendJson