Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c681d5a66e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/apis/LogBox.res
Outdated
| external install: unit => unit = "install" | ||
|
|
||
| @module("react-native") @scope("LogBox") | ||
| external uninstall: unit => unit = "install" |
There was a problem hiding this comment.
Map LogBox.uninstall to the correct native method
uninstall is bound to the JS symbol "install", so LogBox.uninstall() will call install() again instead of removing LogBox hooks. Any caller that relies on cleanup (for example between test runs or runtime reinitialization) will not be able to actually uninstall, which is a functional regression introduced by this binding.
Useful? React with 👍 / 👎.
src/apis/LogBox.res
Outdated
| @@ -1,5 +1,11 @@ | |||
| @module("react-native") @scope("LogBox") | |||
| external ignoreLogs: array<string> => unit = "ignoreLogs" | |||
| external ignoreLogs: array<Js.Re.t> => unit = "ignoreLogs" | |||
There was a problem hiding this comment.
Accept string patterns in LogBox.ignoreLogs
The binding now only accepts array<Js.Re.t>, but LogBox.ignoreLogs supports exact-string matches as well as regex patterns. This change removes a previously supported and commonly used input shape, forcing downstream code using string filters to stop compiling and preventing exact-message suppression from ReScript.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Maybe we can add a separate binding for the regexp version?
src/components/TextInput.res
Outdated
| disableKeyboardShortcuts?: bool, | ||
| cursorColor?: Color.t, | ||
| disableFullscreenUI?: bool, | ||
| dataDetectorTypes?: array<Text.dataDetectorType>, |
There was a problem hiding this comment.
Use TextInput detector type for dataDetectorTypes prop
This file introduces a local dataDetectorType union with TextInput-specific values, but dataDetectorTypes still references Text.dataDetectorType. Since this same commit narrowed Text.dataDetectorType (dropping #address/#calendarEvent), TextInput now rejects valid detector values and the newly added local type is never usable.
Useful? React with 👍 / 👎.
Following #845