Conversation
tassoevan
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRemoves Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
tassoevan
had a problem deploying
to
official_ios_build
GitHub Actions
Error
tassoevan
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
tassoevan
had a problem deploying
to
official_android_build
GitHub Actions
Error
tassoevan
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/lib/methods/serializeAsciiUrl.test.ts (1)
8-8:⚠️ Potential issue | 🟠 Major
jest.mockinsidedescribedoes not produce per-suite platform isolation
jest.mockcalls are hoisted bybabel-jestto the top of the file regardless of where they appear in the source. When two factories are registered for the same module (react-native), Jest uses the last one for the entire file — meaning the iOSdescribeblock also runs under the Android mock ({ Platform: { OS: 'android' } }). The iOS tests therefore do not actually test iOS behaviour.A reliable approach is to use
jest.isolateModulesinside each test so the module registry is reset before each mock is applied:♻️ Proposed fix using `jest.isolateModules`
-describe('Serialize ASCII url on ios', () => { - jest.mock('react-native', () => ({ Platform: { OS: 'ios' } })); - test('ASCII url', () => { - const result = serializeAsciiUrl(ASCIIUrl); - expect(result).toBe(ASCIIUrlSerialized); - }); - test('Non ASCII url', () => { - const result = serializeAsciiUrl(NonASCIIUrl); - expect(result).toBe(NonASCIIUrl); - }); -}); - -describe('Serialize ASCII url on android', () => { - jest.mock('react-native', () => ({ Platform: { OS: 'android' } })); - // By default android converts ASCII addresses - test('ASCII url', () => { - const result = serializeAsciiUrl(ASCIIUrl); - expect(result).toBe(ASCIIUrlSerialized); - }); - test('Non ASCII url', () => { - const result = serializeAsciiUrl(NonASCIIUrl); - expect(result).toBe(NonASCIIUrl); - }); -}); +describe.each([ + ['ios', ASCIIUrlSerialized], + ['android', ASCIIUrlSerialized], // android converts ASCII addresses natively +])('Serialize ASCII url on %s', (os, expectedASCII) => { + let serializeAsciiUrlFn: typeof import('./serializeAsciiUrl').serializeAsciiUrl; + + beforeEach(() => { + jest.resetModules(); + jest.mock('react-native', () => ({ Platform: { OS: os } })); + // eslint-disable-next-line `@typescript-eslint/no-require-imports` + serializeAsciiUrlFn = require('./serializeAsciiUrl').serializeAsciiUrl; + }); + + test('ASCII url', () => { + expect(serializeAsciiUrlFn(ASCIIUrl)).toBe(expectedASCII); + }); + test('Non ASCII url', () => { + expect(serializeAsciiUrlFn(NonASCIIUrl)).toBe(NonASCIIUrl); + }); +});Also applies to: 19-25
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/methods/serializeAsciiUrl.test.ts` at line 8, The jest.mock calls for 'react-native' are hoisted and cause cross-suite leakage; update the tests in serializeAsciiUrl.test.ts to remove per-describe jest.mock usage and instead wrap each test (or beforeEach) in jest.isolateModules so you can call jest.mock('react-native', () => ({ Platform: { OS: 'ios' } })) or the Android variant inside the isolate callback and then require the module under test (the serializeAsciiUrl import) inside that isolateModules block; ensure each test requires the module after mocking so Platform.OS is correctly isolated for tests referencing serializeAsciiUrl.
🧹 Nitpick comments (7)
app/containers/MessageComposer/components/Quotes/Quote.tsx (1)
52-53: Same optional root-cause fix applies as inDiscussionsView/Item.tsx.If
MarkdownPreview'sstyleprop acceptsStyleProp<TextStyle>,style={styles.message}removes the need for the suppress comment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/MessageComposer/components/Quotes/Quote.tsx` around lines 52 - 53, The inline eslint suppression exists because style is passed as a single-element array; update the JSX in Quote.tsx to pass the style directly (use style={styles.message} instead of style={[styles.message]}) and remove the eslint-disable-next-line comment; if TypeScript complains about prop typing, adjust MarkdownPreview's prop type to accept StyleProp<TextStyle> (or cast the value to that type) so the direct style prop is valid—look for the MarkdownPreview prop definition and styles.message to make the change, similar to the fix applied in DiscussionsView/Item.tsx.app/ee/omnichannel/lib/index.ts (1)
56-56: Considerconst _omnichannel = new Omnichannel()for consistency with the PR's underscore convention.The anonymous instantiation is functionally safe (the
EventEmitterholds references to the bound methods, preventing GC of the instance), but it departs from the underscore-prefix pattern applied everywhere else in this PR (_data,_b64,_messageId,_rid). Using_omnichannelmakes the "instantiate for side-effects" intent explicit and keeps the codebase idiom consistent.♻️ Suggested change
-new Omnichannel(); +const _omnichannel = new Omnichannel();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/ee/omnichannel/lib/index.ts` at line 56, The code instantiates Omnichannel anonymously which breaks the PR's underscore-prefix convention; change the anonymous call to a named const (e.g., _omnichannel) so the instance is assigned and follows the pattern used for other module-level side-effect instances; locate the new Omnichannel() invocation in the module (symbol: Omnichannel) and replace it with a const declaration named _omnichannel assigned to the new Omnichannel() instance.app/containers/List/ListRadio.tsx (1)
13-13: Prefer_valueover bare_for the discarded prop.Using
_drops all context about which prop is intentionally ignored._valuesatisfies both TypeScriptnoUnusedParametersand ESLintno-unused-vars(underscore prefix convention) while keeping the original name readable.♻️ Proposed rename
-const ListRadio = ({ value: _, isSelected, ...rest }: IListRadio) => { +const ListRadio = ({ value: _value, isSelected, ...rest }: IListRadio) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/List/ListRadio.tsx` at line 13, In ListRadio change the discarded prop name from "_" to "_value" so the parameter list becomes ({ value: _value, isSelected, ...rest }: IListRadio) — update the ListRadio function signature to use _value instead of _ to satisfy TypeScript/ESLint unused-var conventions while preserving the original prop name context.app/lib/encryption/room.ts (1)
503-503:_ridparameter is structurally dead — consider removing it.
file: TSendFileMessageFileInfoalready includesrid, so the first parameter has never been needed. The rename to_ridsatisfies the linter but leaves every caller supplying a redundant argument.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/encryption/room.ts` at line 503, The function encryptFile currently accepts a redundant first parameter named _rid even though file: TSendFileMessageFileInfo already contains rid; remove the unused _rid parameter from encryptFile's signature and update the implementation to use file.rid wherever needed (refer to encryptFile and TSendFileMessageFileInfo), then update all callers to stop passing the extra rid argument so they call encryptFile(file) and adjust any related type references (e.g., TEncryptFileResult) as needed to match the new signature..eslintrc.js (3)
19-25:'prettier'inextendsis redundant when'plugin:prettier/recommended'is already present.
plugin:prettier/recommendedincludeseslint-config-prettier(which is what'prettier'resolves to). The same duplication appears in the TypeScript override at lines 81–82.♻️ Suggested cleanup
extends: [ '@rocket.chat/eslint-config', 'plugin:react/recommended', 'plugin:react-hooks/recommended', - 'plugin:prettier/recommended', - 'prettier' + 'plugin:prettier/recommended' ],And in the TS override:
extends: [ 'plugin:`@typescript-eslint/recommended`', 'plugin:`@typescript-eslint/eslint-recommended`', '@rocket.chat/eslint-config', - 'plugin:prettier/recommended', - 'prettier' + 'plugin:prettier/recommended' ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.eslintrc.js around lines 19 - 25, The extends array currently includes both "plugin:prettier/recommended" and the redundant "prettier"; remove the duplicate "prettier" entry from the top-level extends list and also remove the duplicate "prettier" entry from the TypeScript override extends to avoid redundant inclusion (locate the extends arrays where "plugin:prettier/recommended" and "prettier" appear and delete only the plain "prettier" entries).
96-100: Several turned-off rules are deprecated in@typescript-eslintv7.
@typescript-eslint/indent,@typescript-eslint/no-extra-parens,@typescript-eslint/no-var-requires, andno-spaced-funcare all deprecated (formatting rules were removed in v6+,no-var-requiresreplaced byno-require-imports). Turning them off is harmless but adds dead configuration. Consider removing them in a follow-up cleanup pass.Also applies to: 108-108, 112-112
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.eslintrc.js around lines 96 - 100, The .eslintrc.js currently disables several deprecated `@typescript-eslint` rules; remove the dead configuration entries for '@typescript-eslint/indent', '@typescript-eslint/no-extra-parens', 'no-spaced-func', and '@typescript-eslint/no-var-requires' (and any duplicate occurrences of those exact rule keys elsewhere in the file) so the config no longer contains deprecated/removed rule names; keep other valid rules like '@typescript-eslint/no-dupe-class-members' and '@typescript-eslint/no-empty-function' unchanged.
45-47:import/no-cyclecan be expensive on large codebases.This rule performs deep dependency graph traversal. In a React Native monorepo of this size, it can noticeably slow down lint runs. Consider setting
maxDepthto bound the cost, or running it only in CI rather than in editor integrations.♻️ Option to bound cycle detection depth
- 'import/no-cycle': 'error', + 'import/no-cycle': ['error', { maxDepth: 5 }],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.eslintrc.js around lines 45 - 47, The 'import/no-cycle' rule in .eslintrc.js is expensive on large monorepos; update its configuration (the 'import/no-cycle' entry) to limit traversal by adding a maxDepth option (e.g., set maxDepth to a small integer like 3–5) or move/override the rule so it's only enforced in CI/editor-independent runs (e.g., enable it in an ESLint CI override or disable it for editor integrations), ensuring you reference the 'import/no-cycle' rule name when making the change.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
app/containers/Avatar/__snapshots__/Avatar.test.tsx.snapis excluded by!**/*.snapapp/containers/markdown/__snapshots__/Markdown.test.tsx.snapis excluded by!**/*.snapapp/containers/message/__snapshots__/Message.test.tsx.snapis excluded by!**/*.snapapp/views/DiscussionsView/__snapshots__/Item.test.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (39)
.eslintignore.eslintrc.js__mocks__/react-native-mmkv.jsapp/containers/ActionSheet/ActionSheet.tsxapp/containers/AudioPlayer/Seek.tsxapp/containers/Avatar/Avatar.stories.tsxapp/containers/Avatar/AvatarContainer.tsxapp/containers/Avatar/AvatarWithEdit.tsxapp/containers/List/ListRadio.tsxapp/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/helpers/getMentionRegexp.test.jsapp/containers/UnreadBadge/getUnreadStyle.test.jsapp/containers/markdown/components/emoji/Emoji.tsxapp/containers/message/Components/Attachments/Reply.tsxapp/ee/omnichannel/lib/index.tsapp/ee/omnichannel/views/QueueListView.tsxapp/lib/database/utils.test.tsapp/lib/encryption/encryption.tsapp/lib/encryption/helpers/base64-js/index.tsapp/lib/encryption/room.tsapp/lib/helpers/formatText.tsapp/lib/hooks/useShortnameToUnicode/index.tsxapp/lib/methods/search.tsapp/lib/methods/serializeAsciiUrl.test.tsapp/sagas/createChannel.jsapp/sagas/createDiscussion.jsapp/sagas/deepLinking.jsapp/sagas/login.jsapp/sagas/rooms.jsapp/sagas/state.jsapp/views/CreateChannelView/RoomSettings/SwitchItemEncrypted.test.tsxapp/views/DiscussionsView/DiscussionDetails.tsxapp/views/DiscussionsView/Item.tsxapp/views/RoomView/Banner.tsxapp/views/SelectServerView.tsxapp/views/ThreadMessagesView/Item.tsxpackage.jsonscripts/build-icon-set.jstsconfig.json
💤 Files with no reviewable changes (1)
- .eslintignore
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-02-05T13:55:00.974Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:00.974Z
Learning: In this repository, the dependency on react-native-image-crop-picker should reference the RocketChat fork (RocketChat/react-native-image-crop-picker) with explicit commit pins, not the upstream ivpusic/react-native-image-crop-picker. Update package.json dependencies (and any lockfile) to point to the fork URL and a specific commit, ensuring edge-to-edge Android fixes are included. This pattern should apply to all package.json files in the repo that declare this dependency.
Applied to files:
package.json
🧬 Code graph analysis (6)
app/sagas/state.js (1)
app/lib/notifications/index.ts (1)
checkPendingNotification(100-135)
app/lib/encryption/encryption.ts (1)
app/lib/encryption/definitions.ts (1)
TDecryptFile(18-23)
app/views/SelectServerView.tsx (1)
app/containers/ServerItem/index.tsx (1)
ROW_HEIGHT(12-12)
app/sagas/createDiscussion.js (2)
app/sagas/createChannel.js (1)
subCollection(62-62)app/sagas/rooms.js (1)
subCollection(52-52)
app/lib/encryption/room.ts (2)
app/definitions/IUpload.ts (1)
TSendFileMessageFileInfo(26-29)app/lib/encryption/definitions.ts (1)
TEncryptFileResult(10-14)
app/containers/Avatar/AvatarWithEdit.tsx (1)
app/containers/message/MessageAvatar.tsx (1)
AvatarContainer(13-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🔇 Additional comments (35)
app/lib/helpers/formatText.ts (1)
5-5: LGTM!Renaming
match→_matchcorrectly signals the parameter is intentionally unused, consistent with the lint rule being enforced across the PR. No behavioral change.app/lib/database/utils.test.ts (1)
13-21: LGTM — clean consolidation intotest.each.The array-of-arrays format,
%stitle interpolation,_prefix for the unused label parameter, and length-based'_'.repeat(disallowedChars.length)expectation (which correctly handles_being both a disallowed char and the replacement character) are all correct.app/lib/methods/serializeAsciiUrl.test.ts (1)
22-25: LGTM — placeholder replaced with the correct constantChanging the expected value from the stale
'filename.png'placeholder toASCIIUrlSerializedcorrectly reflects the behaviour described in the comment above (Android serializes ASCII URLs to their Punycode form).app/sagas/createDiscussion.js (1)
26-29: LGTM — parentheses removal is stylistically clean.app/lib/methods/search.ts (1)
141-142: LGTM —_resolverename correctly signals the unused parameter.The executor only assigns
rejecttodebounce;resolvewas and remains uncalled. The cancellation pattern is sound.app/sagas/login.js (1)
61-64: LGTM — consistent single-parameter arrow function style across WatermelonDB callbacks.app/lib/hooks/useShortnameToUnicode/index.tsx (1)
36-48: LGTM —_m1rename correctly reflects that capture group 1 is unused.
entire(position 0) already covers the full match;_m1(position 1) would be its duplicate, so the underscore prefix is appropriate.app/sagas/state.js (1)
31-33: LGTM.app/containers/message/Components/Attachments/Reply.tsx (1)
264-271: LGTM — removing the single-element array from the outerViewis correct.The outer
View(container) only needsstyles.backdrop; the innerView(lines 266–269) correctly retains the two-element array for dynamic opacity/color. The treatment is consistent with React Native'sStyleProp<ViewStyle>.app/views/DiscussionsView/Item.tsx (1)
80-81: The current implementation with the eslint-disable comment is correct and necessary.The
styleprop inMarkdownPreviewis explicitly typed asTextStyle[](seeapp/containers/markdown/components/Preview.tsxline 13), so the array wrapperstyle={[styles.markdown]}is required by the component's type signature. The component spreads this array into a larger style array (line 30), confirming the array type is intentional. The eslint-disable comment correctly suppresses a false-positive lint warning in this case—removing the array would violate the component's type contract.app/ee/omnichannel/views/QueueListView.tsx (1)
70-77: LGTM.Renaming
data→_datacorrectly communicates that the first parameter is intentionally unused, satisfying the newly enablednoUnusedParametersrule without any behavioral change.app/sagas/createChannel.js (1)
64-67: LGTM.Removing the parentheses around the single
sparameter is idiomatic and semantically identical.app/sagas/deepLinking.js (1)
49-56: LGTM.Removing the parentheses around the single
resolveparameter is a no-op style change consistent with the rest of the PR.app/sagas/rooms.js (1)
21-23: LGTM.All six single-parameter arrow functions consistently drop their optional parentheses. No behavioral change in any case.
Also applies to: 78-82, 83-101, 102-109, 111-115, 117-127
app/containers/UnreadBadge/getUnreadStyle.test.js (1)
5-5: LGTM.Removing parentheses around the single
themeparameter is a consistent style-only change.app/views/ThreadMessagesView/Item.tsx (1)
91-92: The ESLint disable comment is appropriate and necessary here.
MarkdownPreview'sstyleprop is explicitly typed asTextStyle[](an array), not as a scalar orStyleProp<TextStyle>. The component spreads the style into an array:style={[styles.text, { color: ... }, ...style]}. Therefore,style={[styles.markdown]}is the correct way to pass a value, and the ESLint suppress comment is the right approach—no refactoring needed.app/views/SelectServerView.tsx (1)
18-18: LGTM — correct underscore-prefix convention for the unuseddataparameter.tsconfig.json (1)
38-39: Good strictness uplift — enablingnoUnusedLocalsandnoUnusedParameters.These flags are the root driver for all the
_-prefixed parameter renames throughout the rest of this PR, so the codebase is already consistent with them.app/views/DiscussionsView/DiscussionDetails.tsx (1)
45-45: LGTM — removing the single-item style array wrapper is functionally equivalent in React Native.scripts/build-icon-set.js (1)
4-4: LGTM — explicit block body is functionally equivalent and more readable.app/lib/encryption/helpers/base64-js/index.ts (1)
47-48: LGTM —_b64is correctly marked unused; the function body only needsvalidLenandplaceHoldersLen.app/containers/markdown/components/emoji/Emoji.tsx (1)
72-72: LGTM — removing the single-element style array is functionally equivalent.app/lib/encryption/encryption.ts (1)
599-609: LGTM —_messageIdcorrectly signals the unused first parameter; implementation aligns with theTDecryptFilecontract.app/containers/Avatar/AvatarContainer.tsx (1)
74-76: LGTM — idiomatic JSX children pattern.Switching from
children={children}prop syntax to nested JSX is a style improvement with no behavioral difference.app/containers/MessageComposer/helpers/getMentionRegexp.test.js (1)
32-32: LGTM — quote normalization with no behavioral change.app/containers/AudioPlayer/Seek.tsx (1)
68-71: LGTM — unused parameter correctly prefixed.app/containers/Avatar/AvatarWithEdit.tsx (1)
67-69: LGTM — consistent with the children-as-JSX pattern adopted inAvatarContainer.tsx.app/containers/Avatar/Avatar.stories.tsx (1)
67-67: LGTM — unnecessary single-element array wrapper correctly removed.React Native's
StylePropaccepts a registered style number directly; the array wrapper was redundant.app/containers/ActionSheet/ActionSheet.tsx (1)
171-173: LGTM — same idiomatic JSX children pattern applied consistently.package.json (2)
177-178: Version range change from^to~for TypeScript ESLint packages.Switching from
^7.4.0to~7.18.0tightens the allowed range to patch-only updates, which is a sensible choice for linting tooling stability. 7.18.0 is the latest 7.x release, so this effectively pins at the final minor.
184-192: Botheslint-plugin-react-hooksv7.0.1 andeslint-plugin-react-nativev5.x are compatible with ESLint 8.57.0.
eslint-plugin-react-hooks7.0.1 declares ESLint peer dependency^8.0.0-0 || ^9.0.0, confirming ESLint 8 support.eslint-plugin-react-native5.x is also compatible with ESLint 8 using the classic.eslintrc*format. The three rules referenced (react-hooks/set-state-in-effect,react-hooks/immutability,react-hooks/refs) are already in use at lines 60–62 with no apparent issues. The ESLint 9 flat config migration is irrelevant for the current ESLint 8 setup.Likely an incorrect or invalid review comment.
__mocks__/react-native-mmkv.js (1)
87-87: Stylistic-only changes — LGTM.All changes remove redundant parentheses around single arrow function parameters, consistent with the Prettier/lint enforcement goals of this PR.
Also applies to: 105-105, 113-113, 129-129, 137-137, 153-153, 161-161, 180-180, 189-189
.eslintrc.js (3)
101-107: Good: strict unused-vars enforcement with underscore escape hatch.Configuring
args: 'all'withargsIgnorePattern: '^_'is a solid pattern — it catches genuinely unused parameters while allowing intentional placeholders. This aligns well with thetsconfig.jsonchanges enablingnoUnusedParameters.
14-16: Good structural improvements: auto-detecting React version and scoping environments.Auto-detecting the React version (line 15) eliminates the common "React version not specified" warning. The test-file override (line 131–133) and React Native env scoping (lines 134–139) properly isolate concerns.
Also applies to: 131-139
60-62: No action needed. All three rule names (react-hooks/set-state-in-effect,react-hooks/immutability,react-hooks/refs) are valid and confirmed to exist ineslint-plugin-react-hooksv7.0.0 as part of the React Compiler rules set enabled by default.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/views/CreateChannelView/RoomSettings/SwitchItemEncrypted.test.tsx`:
- Around line 59-61: The test is sending an object instead of a boolean to the
Switch handler and asserting on the returned object rather than the call
argument; update the fireEvent call that targets
screen.getByTestId(testEncrypted.testSwitchID) to pass a plain boolean (true) —
i.e. fireEvent(component, 'valueChange', true) — and replace the tautological
expect(onPressMock).toHaveReturnedWith(...) with
expect(onPressMock).toHaveBeenCalledWith(true) so the test verifies the callback
was invoked with a boolean as the Switch would in real usage (references:
testEncrypted, testSwitchID, fireEvent, onPressMock).
---
Outside diff comments:
In `@app/lib/methods/serializeAsciiUrl.test.ts`:
- Line 8: The jest.mock calls for 'react-native' are hoisted and cause
cross-suite leakage; update the tests in serializeAsciiUrl.test.ts to remove
per-describe jest.mock usage and instead wrap each test (or beforeEach) in
jest.isolateModules so you can call jest.mock('react-native', () => ({ Platform:
{ OS: 'ios' } })) or the Android variant inside the isolate callback and then
require the module under test (the serializeAsciiUrl import) inside that
isolateModules block; ensure each test requires the module after mocking so
Platform.OS is correctly isolated for tests referencing serializeAsciiUrl.
---
Duplicate comments:
In `@app/views/RoomView/Banner.tsx`:
- Around line 33-34: The Banner component is passing style as an array wrapper
to <MarkdownPreview> which triggered an eslint suppression; remove the array
wrapper so the prop is passed as style={styles.bannerText} (and delete the
eslint-disable-next-line comment) and, if necessary, update MarkdownPreview's
prop type to accept StyleProp<TextStyle> so this direct style object is valid;
locate the usage in Banner.tsx (MarkdownPreview msg={text}
style={[styles.bannerText]}) and change accordingly.
---
Nitpick comments:
In @.eslintrc.js:
- Around line 19-25: The extends array currently includes both
"plugin:prettier/recommended" and the redundant "prettier"; remove the duplicate
"prettier" entry from the top-level extends list and also remove the duplicate
"prettier" entry from the TypeScript override extends to avoid redundant
inclusion (locate the extends arrays where "plugin:prettier/recommended" and
"prettier" appear and delete only the plain "prettier" entries).
- Around line 96-100: The .eslintrc.js currently disables several deprecated
`@typescript-eslint` rules; remove the dead configuration entries for
'@typescript-eslint/indent', '@typescript-eslint/no-extra-parens',
'no-spaced-func', and '@typescript-eslint/no-var-requires' (and any duplicate
occurrences of those exact rule keys elsewhere in the file) so the config no
longer contains deprecated/removed rule names; keep other valid rules like
'@typescript-eslint/no-dupe-class-members' and
'@typescript-eslint/no-empty-function' unchanged.
- Around line 45-47: The 'import/no-cycle' rule in .eslintrc.js is expensive on
large monorepos; update its configuration (the 'import/no-cycle' entry) to limit
traversal by adding a maxDepth option (e.g., set maxDepth to a small integer
like 3–5) or move/override the rule so it's only enforced in
CI/editor-independent runs (e.g., enable it in an ESLint CI override or disable
it for editor integrations), ensuring you reference the 'import/no-cycle' rule
name when making the change.
In `@app/containers/List/ListRadio.tsx`:
- Line 13: In ListRadio change the discarded prop name from "_" to "_value" so
the parameter list becomes ({ value: _value, isSelected, ...rest }: IListRadio)
— update the ListRadio function signature to use _value instead of _ to satisfy
TypeScript/ESLint unused-var conventions while preserving the original prop name
context.
In `@app/containers/MessageComposer/components/Quotes/Quote.tsx`:
- Around line 52-53: The inline eslint suppression exists because style is
passed as a single-element array; update the JSX in Quote.tsx to pass the style
directly (use style={styles.message} instead of style={[styles.message]}) and
remove the eslint-disable-next-line comment; if TypeScript complains about prop
typing, adjust MarkdownPreview's prop type to accept StyleProp<TextStyle> (or
cast the value to that type) so the direct style prop is valid—look for the
MarkdownPreview prop definition and styles.message to make the change, similar
to the fix applied in DiscussionsView/Item.tsx.
In `@app/ee/omnichannel/lib/index.ts`:
- Line 56: The code instantiates Omnichannel anonymously which breaks the PR's
underscore-prefix convention; change the anonymous call to a named const (e.g.,
_omnichannel) so the instance is assigned and follows the pattern used for other
module-level side-effect instances; locate the new Omnichannel() invocation in
the module (symbol: Omnichannel) and replace it with a const declaration named
_omnichannel assigned to the new Omnichannel() instance.
In `@app/lib/encryption/room.ts`:
- Line 503: The function encryptFile currently accepts a redundant first
parameter named _rid even though file: TSendFileMessageFileInfo already contains
rid; remove the unused _rid parameter from encryptFile's signature and update
the implementation to use file.rid wherever needed (refer to encryptFile and
TSendFileMessageFileInfo), then update all callers to stop passing the extra rid
argument so they call encryptFile(file) and adjust any related type references
(e.g., TEncryptFileResult) as needed to match the new signature.
fd66233 to
800e82b
Compare
tassoevan
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/lib/encryption/encryption.ts (1)
599-599: LGTM — unusedmessageIdparameter correctly prefixed.
_messageIdis never used in the body; decryption depends only onpath,encryption, andoriginalChecksum. The value is still correctly threaded throughaddFileToDecryptFileQueue→processFileQueue→decryptFile, so the queue contract is unaffected.For completeness, the
messageIdlabel in theTDecryptFiletype alias (app/lib/encryption/definitions.ts:17) could be updated to_messageIdto stay consistent with the implementation convention, though TypeScript does not enforce name parity between a type alias and its implementing assignment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/encryption/encryption.ts` at line 599, The TDecryptFile type alias's first parameter name should be renamed from messageId to _messageId in app/lib/encryption/definitions.ts to match the implementation's unused parameter naming; update the parameter label in the type alias (TDecryptFile) to _messageId so it aligns with decryptFile's signature and maintainers' convention—no behavioural changes needed, and calls through addFileToDecryptFileQueue and processFileQueue remain unaffected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/lib/encryption/encryption.ts`:
- Line 599: The TDecryptFile type alias's first parameter name should be renamed
from messageId to _messageId in app/lib/encryption/definitions.ts to match the
implementation's unused parameter naming; update the parameter label in the type
alias (TDecryptFile) to _messageId so it aligns with decryptFile's signature and
maintainers' convention—no behavioural changes needed, and calls through
addFileToDecryptFileQueue and processFileQueue remain unaffected.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
app/containers/Avatar/__snapshots__/Avatar.test.tsx.snapis excluded by!**/*.snapapp/containers/markdown/__snapshots__/Markdown.test.tsx.snapis excluded by!**/*.snapapp/containers/message/__snapshots__/Message.test.tsx.snapis excluded by!**/*.snapapp/views/DiscussionsView/__snapshots__/Item.test.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (39)
.eslintignore.eslintrc.js__mocks__/react-native-mmkv.jsapp/containers/ActionSheet/ActionSheet.tsxapp/containers/AudioPlayer/Seek.tsxapp/containers/Avatar/Avatar.stories.tsxapp/containers/Avatar/AvatarContainer.tsxapp/containers/Avatar/AvatarWithEdit.tsxapp/containers/List/ListRadio.tsxapp/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/helpers/getMentionRegexp.test.jsapp/containers/UnreadBadge/getUnreadStyle.test.jsapp/containers/markdown/components/emoji/Emoji.tsxapp/containers/message/Components/Attachments/Reply.tsxapp/ee/omnichannel/lib/index.tsapp/ee/omnichannel/views/QueueListView.tsxapp/lib/database/utils.test.tsapp/lib/encryption/encryption.tsapp/lib/encryption/helpers/base64-js/index.tsapp/lib/encryption/room.tsapp/lib/helpers/formatText.tsapp/lib/hooks/useShortnameToUnicode/index.tsxapp/lib/methods/search.tsapp/lib/methods/serializeAsciiUrl.test.tsapp/sagas/createChannel.jsapp/sagas/createDiscussion.jsapp/sagas/deepLinking.jsapp/sagas/login.jsapp/sagas/rooms.jsapp/sagas/state.jsapp/views/CreateChannelView/RoomSettings/SwitchItemEncrypted.test.tsxapp/views/DiscussionsView/DiscussionDetails.tsxapp/views/DiscussionsView/Item.tsxapp/views/RoomView/Banner.tsxapp/views/SelectServerView.tsxapp/views/ThreadMessagesView/Item.tsxpackage.jsonscripts/build-icon-set.jstsconfig.json
💤 Files with no reviewable changes (1)
- .eslintignore
✅ Files skipped from review due to trivial changes (3)
- app/lib/methods/search.ts
- app/sagas/rooms.js
- app/views/SelectServerView.tsx
🚧 Files skipped from review as they are similar to previous changes (26)
- app/sagas/deepLinking.js
- app/lib/methods/serializeAsciiUrl.test.ts
- app/views/DiscussionsView/Item.tsx
- app/sagas/login.js
- app/containers/UnreadBadge/getUnreadStyle.test.js
- app/sagas/createChannel.js
- app/views/RoomView/Banner.tsx
- tsconfig.json
- mocks/react-native-mmkv.js
- app/lib/database/utils.test.ts
- app/containers/MessageComposer/components/Quotes/Quote.tsx
- app/ee/omnichannel/lib/index.ts
- app/sagas/state.js
- app/lib/encryption/helpers/base64-js/index.ts
- app/containers/List/ListRadio.tsx
- app/containers/message/Components/Attachments/Reply.tsx
- app/views/DiscussionsView/DiscussionDetails.tsx
- app/views/CreateChannelView/RoomSettings/SwitchItemEncrypted.test.tsx
- app/containers/Avatar/AvatarWithEdit.tsx
- app/containers/AudioPlayer/Seek.tsx
- app/sagas/createDiscussion.js
- app/containers/ActionSheet/ActionSheet.tsx
- app/containers/MessageComposer/helpers/getMentionRegexp.test.js
- app/containers/markdown/components/emoji/Emoji.tsx
- app/views/ThreadMessagesView/Item.tsx
- app/lib/helpers/formatText.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-02-05T13:55:00.974Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:00.974Z
Learning: In this repository, the dependency on react-native-image-crop-picker should reference the RocketChat fork (RocketChat/react-native-image-crop-picker) with explicit commit pins, not the upstream ivpusic/react-native-image-crop-picker. Update package.json dependencies (and any lockfile) to point to the fork URL and a specific commit, ensuring edge-to-edge Android fixes are included. This pattern should apply to all package.json files in the repo that declare this dependency.
Applied to files:
package.json
🧬 Code graph analysis (2)
app/lib/encryption/encryption.ts (1)
app/lib/encryption/definitions.ts (1)
TDecryptFile(18-23)
app/lib/encryption/room.ts (2)
app/definitions/IUpload.ts (1)
TSendFileMessageFileInfo(26-29)app/lib/encryption/definitions.ts (1)
TEncryptFileResult(10-14)
🔇 Additional comments (8)
app/ee/omnichannel/views/QueueListView.tsx (1)
70-70: Nice cleanup: intentional unused parameter is explicit.The
_datarename keeps theFlatListcallback signature while making non-usage clear for linting/readability.app/lib/hooks/useShortnameToUnicode/index.tsx (1)
36-36: Good lint-safe rename for unused capture group.Renaming to
_m1clearly marks it as intentionally unused while keeping them2/m3capture positions unchanged.app/containers/Avatar/AvatarContainer.tsx (1)
74-76: LGTM — idiomatic JSX child composition.Converting from
children={children}prop to nested JSX content is semantically equivalent in React and is the conventional pattern. No functional or type-safety regression.app/containers/Avatar/Avatar.stories.tsx (1)
67-67: LGTM — single-element style array correctly unwrapped.React Native's
styleprop accepts a plain style object directly; removing the redundant single-element array wrapper is valid and marginally more efficient (avoids an ephemeral array allocation on every render).app/lib/encryption/room.ts (1)
503-503: LGTM — unusedridparameter correctly prefixed.
_ridis never referenced inside the method body;this.roomIdis the source of truth for the room ID withinEncryptionRoom. The rename accurately signals intent.scripts/build-icon-set.js (1)
4-4: Reducer refactor preserves behavior.The block-bodied reducer still returns the same sorted object output while improving readability.
.eslintrc.js (1)
19-33: ESLint config refactor is clean and well-scoped.The updated base rules plus targeted TypeScript, Jest, and React Native overrides are organized clearly and align with the stated lint-consistency objective.
Also applies to: 45-73, 74-139
package.json (1)
177-178: No action needed. The@typescript-eslintversions are compatible with the repository's TypeScript version.The npm peer dependency metadata shows no TypeScript version constraint for
@typescript-eslint7.18.0, indicating compatibility with TypeScript 5.9.3. The concern about parser/lint breakage is unfounded.Likely an incorrect or invalid review comment.
tassoevan
force-pushed
the
chore/stylistic-changes-1
branch
from
800e82b to
dd86d07
Compare
tassoevan
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/views/CreateChannelView/RoomSettings/SwitchItemEncrypted.test.tsx (1)
60-61:fireEventpayload and assertion are still incorrect.Both issues flagged in the prior review are unaddressed:
fireEvent(component, 'valueChange', { value: true })— passes an object to a handler typed as(value: boolean) => value; the argument should be the plain booleantrue.toHaveReturnedWith({ value: !testEncrypted.encrypted })— since the mock returns its argument and the argument is{ value: true }, this assertion is tautological and tests nothing meaningful. UsetoHaveBeenCalledWith(true)instead.🛠️ Proposed fix
- fireEvent(component, 'valueChange', { value: true }); - expect(onPressMock).toHaveReturnedWith({ value: !testEncrypted.encrypted }); + fireEvent(component, 'valueChange', true); + expect(onPressMock).toHaveBeenCalledWith(true);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/CreateChannelView/RoomSettings/SwitchItemEncrypted.test.tsx` around lines 60 - 61, The test fires the wrong payload and asserts the wrong behavior: change the fireEvent call in SwitchItemEncrypted.test to pass a plain boolean (true) to the handler (i.e., fireEvent(component, 'valueChange', true)) and replace the tautological toHaveReturnedWith check with a meaningful call assertion: assert that the mock onPress handler was called with true (i.e., toHaveBeenCalledWith(true)); refer to the test's fireEvent invocation and the onPressMock / testEncrypted.encrypted references to locate the lines to change.
🧹 Nitpick comments (1)
.eslintrc.js (1)
19-25: Redundant'prettier'entry inextends.
plugin:prettier/recommended(line 23) already incorporateseslint-config-prettieras its first extend, so the trailing'prettier'on line 24 has no effect and can be removed. The same duplication exists inside the TypeScript override (line 82).🧹 Proposed cleanup
extends: [ '@rocket.chat/eslint-config', 'plugin:react/recommended', 'plugin:react-hooks/recommended', 'plugin:prettier/recommended', - 'prettier' ],extends: [ 'plugin:`@typescript-eslint/recommended`', 'plugin:`@typescript-eslint/eslint-recommended`', '@rocket.chat/eslint-config', 'plugin:prettier/recommended', - 'prettier' ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.eslintrc.js around lines 19 - 25, Remove the redundant 'prettier' entry from the top-level extends array and the TypeScript override extends: both already include 'plugin:prettier/recommended' which internally applies eslint-config-prettier, so delete the duplicated 'prettier' string from the extends arrays (look for the top-level extends and the TypeScript override where 'plugin:prettier/recommended' and 'prettier' are listed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/views/CreateChannelView/RoomSettings/SwitchItemEncrypted.test.tsx`:
- Around line 60-61: The test fires the wrong payload and asserts the wrong
behavior: change the fireEvent call in SwitchItemEncrypted.test to pass a plain
boolean (true) to the handler (i.e., fireEvent(component, 'valueChange', true))
and replace the tautological toHaveReturnedWith check with a meaningful call
assertion: assert that the mock onPress handler was called with true (i.e.,
toHaveBeenCalledWith(true)); refer to the test's fireEvent invocation and the
onPressMock / testEncrypted.encrypted references to locate the lines to change.
---
Nitpick comments:
In @.eslintrc.js:
- Around line 19-25: Remove the redundant 'prettier' entry from the top-level
extends array and the TypeScript override extends: both already include
'plugin:prettier/recommended' which internally applies eslint-config-prettier,
so delete the duplicated 'prettier' string from the extends arrays (look for the
top-level extends and the TypeScript override where
'plugin:prettier/recommended' and 'prettier' are listed).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
app/containers/Avatar/__snapshots__/Avatar.test.tsx.snapis excluded by!**/*.snapapp/containers/markdown/__snapshots__/Markdown.test.tsx.snapis excluded by!**/*.snapapp/containers/message/__snapshots__/Message.test.tsx.snapis excluded by!**/*.snapapp/views/DiscussionsView/__snapshots__/Item.test.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (39)
.eslintignore.eslintrc.js__mocks__/react-native-mmkv.jsapp/containers/ActionSheet/ActionSheet.tsxapp/containers/AudioPlayer/Seek.tsxapp/containers/Avatar/Avatar.stories.tsxapp/containers/Avatar/AvatarContainer.tsxapp/containers/Avatar/AvatarWithEdit.tsxapp/containers/List/ListRadio.tsxapp/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/helpers/getMentionRegexp.test.jsapp/containers/UnreadBadge/getUnreadStyle.test.jsapp/containers/markdown/components/emoji/Emoji.tsxapp/containers/message/Components/Attachments/Reply.tsxapp/ee/omnichannel/lib/index.tsapp/ee/omnichannel/views/QueueListView.tsxapp/lib/database/utils.test.tsapp/lib/encryption/encryption.tsapp/lib/encryption/helpers/base64-js/index.tsapp/lib/encryption/room.tsapp/lib/helpers/formatText.tsapp/lib/hooks/useShortnameToUnicode/index.tsxapp/lib/methods/search.tsapp/lib/methods/serializeAsciiUrl.test.tsapp/sagas/createChannel.jsapp/sagas/createDiscussion.jsapp/sagas/deepLinking.jsapp/sagas/login.jsapp/sagas/rooms.jsapp/sagas/state.jsapp/views/CreateChannelView/RoomSettings/SwitchItemEncrypted.test.tsxapp/views/DiscussionsView/DiscussionDetails.tsxapp/views/DiscussionsView/Item.tsxapp/views/RoomView/Banner.tsxapp/views/SelectServerView.tsxapp/views/ThreadMessagesView/Item.tsxpackage.jsonscripts/build-icon-set.jstsconfig.json
💤 Files with no reviewable changes (1)
- .eslintignore
✅ Files skipped from review due to trivial changes (1)
- app/sagas/createChannel.js
🚧 Files skipped from review as they are similar to previous changes (23)
- app/sagas/state.js
- app/views/RoomView/Banner.tsx
- app/containers/UnreadBadge/getUnreadStyle.test.js
- tsconfig.json
- app/containers/MessageComposer/components/Quotes/Quote.tsx
- app/sagas/deepLinking.js
- app/containers/List/ListRadio.tsx
- app/containers/Avatar/Avatar.stories.tsx
- app/lib/methods/serializeAsciiUrl.test.ts
- app/ee/omnichannel/lib/index.ts
- app/sagas/createDiscussion.js
- app/views/ThreadMessagesView/Item.tsx
- app/containers/markdown/components/emoji/Emoji.tsx
- app/containers/AudioPlayer/Seek.tsx
- app/containers/Avatar/AvatarWithEdit.tsx
- app/lib/encryption/encryption.ts
- app/lib/helpers/formatText.ts
- app/lib/database/utils.test.ts
- app/sagas/login.js
- app/views/DiscussionsView/Item.tsx
- app/views/SelectServerView.tsx
- scripts/build-icon-set.js
- package.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
Learnt from: OtavioStasiak
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6499
File: app/containers/ServerItem/index.tsx:34-36
Timestamp: 2025-12-17T15:56:22.578Z
Learning: In the Rocket.Chat React Native codebase, for radio button components on iOS, include the selection state ("Selected"/"Unselected") in the accessibilityLabel instead of using accessibilityState={{ checked: hasCheck }}, because iOS VoiceOver has known issues with accessibilityRole="radio" + accessibilityState that prevent correct state announcement.
Applied to files:
app/views/CreateChannelView/RoomSettings/SwitchItemEncrypted.test.tsx
🧬 Code graph analysis (3)
app/lib/methods/search.ts (2)
app/lib/encryption/helpers/deferred.ts (1)
reject(36-38)app/lib/methods/helpers/debounce.ts (1)
debounce(3-25)
app/lib/encryption/room.ts (2)
app/definitions/IUpload.ts (1)
TSendFileMessageFileInfo(26-29)app/lib/encryption/definitions.ts (1)
TEncryptFileResult(10-14)
app/ee/omnichannel/views/QueueListView.tsx (1)
app/definitions/IRoom.ts (1)
IOmnichannelRoom(107-143)
🔇 Additional comments (16)
app/ee/omnichannel/views/QueueListView.tsx (1)
70-70: LGTM!Renaming the unused
dataparameter to_datais the correct convention to satisfy the@typescript-eslint/no-unused-varsrule for intentionally-unused parameters. The function body and behavior are unchanged.__mocks__/react-native-mmkv.js (1)
87-87: Stylistic changes look good.All modifications are consistent parentheses removal from single-parameter arrow functions, aligning with the Prettier/ESLint style enforcement described in the PR objectives. No behavioral changes.
Also applies to: 105-105, 113-113, 129-137, 153-161, 180-189
app/lib/encryption/helpers/base64-js/index.ts (1)
47-47: Intentional unused parameter is clearly expressed.Line 47 is a clean, low-risk lint-alignment change; behavior remains unchanged.
app/lib/encryption/room.ts (1)
503-503: Good lint-oriented rename for an intentionally unused argument.Line 503 keeps the API shape while making unused intent explicit; this looks correct.
app/lib/methods/search.ts (1)
141-141: LGTM — correct lint-driven rename.Renaming
resolve→_resolveaccurately signals that the resolve callback is intentionally unused; onlyrejectis captured and assigned todebouncefor the cancellation race. No behavioral change.app/views/CreateChannelView/RoomSettings/SwitchItemEncrypted.test.tsx (1)
59-59:getByTestIdis the right query for this test.Switching from
queryByTestIdtogetByTestIdis correct here — the component is expected to be present, andgetByTestIdwill surface a meaningful assertion failure (vs. anull-reference error later) if the element is missing.app/containers/ActionSheet/ActionSheet.tsx (1)
171-173: LGTM!Converting from
children={data?.children}prop to nested JSX children is idiomatic React and functionally equivalent;data?.childrenbeingundefinedon the initial empty-state still renders nothing correctly.app/containers/Avatar/AvatarContainer.tsx (1)
74-76: TheAvatarcomponent's props interface (IAvatar) already explicitly includeschildren?: React.ReactElement | null;at the interface definition level. The component properly destructures and renders this prop, so the refactor is fully type-safe with no TypeScript errors.app/containers/message/Components/Attachments/Reply.tsx (1)
264-264: LGTM — correct unwrap of single-element style array.Aligns with the newly enforced
react-native/no-single-element-style-arraysrule; no runtime behaviour change.app/lib/hooks/useShortnameToUnicode/index.tsx (1)
36-48: LGTM —_m1prefix correctly marks the unused capture group.app/views/DiscussionsView/DiscussionDetails.tsx (1)
45-45: LGTM — single-element style array correctly unwrapped.app/sagas/rooms.js (1)
21-21: LGTM — cosmetic-only removal of redundant parentheses around single arrow-function parameters throughout the file.app/containers/MessageComposer/helpers/getMentionRegexp.test.js (1)
32-32: LGTM — quote style change to avoid backslash escaping; string value unchanged..eslintrc.js (3)
66-67: LGTM —react-native/no-single-element-style-arraysandreact-native/no-unused-stylesaterrorlevel are well-motivated.These two new rules directly drive the mechanical fixes across
Reply.tsx,DiscussionDetails.tsx, and the other style-array unwraps in this PR.
60-62: No action needed — all three React Compiler rules are properly supported.The installed
eslint-plugin-react-hooksv7.0.1 fully supportsset-state-in-effect,immutability, andrefsrules. These rules are properly defined in the plugin bundle with complete metadata and will function as configured in.eslintrc.jslines 60-62.Likely an incorrect or invalid review comment.
101-107: The ESLint rule configuration withargs: 'all'andargsIgnorePattern: '^_'has been properly implemented throughout the codebase. The example cited in the review—the_m1parameter inuseShortnameToUnicode—is correctly prefixed with an underscore on line 41. No remaining violations of this rule were found across the TypeScript files in the app directory. All unused callback parameters already follow the required pattern.
tassoevan
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
tassoevan
had a problem deploying
to
official_android_build
GitHub Actions
Error
tassoevan
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
tassoevan
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
tassoevan
had a problem deploying
to
official_ios_build
GitHub Actions
Error
tassoevan
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/views/DiscussionsView/Item.tsx (1)
80-81: The ESLint disable comment appears unnecessary.Since
styles.markdownis passed directly as a single style object (not wrapped in an array), thereact-native/no-single-element-style-arraysrule shouldn't trigger. With theStyleProp<TextStyle>typing change inPreview.tsx, the lint suppression can be removed.🧹 Suggested cleanup
- {/* eslint-disable-next-line react-native/no-single-element-style-arrays */} - {username ? <MarkdownPreview msg={makeThreadName(item)} numberOfLines={2} style={styles.markdown} /> : null} + {username ? <MarkdownPreview msg={makeThreadName(item)} numberOfLines={2} style={styles.markdown} /> : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/DiscussionsView/Item.tsx` around lines 80 - 81, Remove the unnecessary ESLint suppression: delete the comment "// eslint-disable-next-line react-native/no-single-element-style-arrays" before the JSX that renders MarkdownPreview since styles.markdown is passed as a single style object and the StyleProp<TextStyle> change in Preview.tsx already prevents the rule from triggering; update the line that renders MarkdownPreview (the JSX using MarkdownPreview, makeThreadName(item), and styles.markdown) to have no eslint-disable comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/views/DiscussionsView/Item.tsx`:
- Around line 80-81: Remove the unnecessary ESLint suppression: delete the
comment "// eslint-disable-next-line
react-native/no-single-element-style-arrays" before the JSX that renders
MarkdownPreview since styles.markdown is passed as a single style object and the
StyleProp<TextStyle> change in Preview.tsx already prevents the rule from
triggering; update the line that renders MarkdownPreview (the JSX using
MarkdownPreview, makeThreadName(item), and styles.markdown) to have no
eslint-disable comment.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/containers/markdown/components/Preview.tsxapp/views/DiscussionsView/Item.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
🧬 Code graph analysis (2)
app/views/DiscussionsView/Item.tsx (2)
app/lib/methods/helpers/room.ts (1)
makeThreadName(69-70)app/sagas/deepLinking.js (1)
item(71-76)
app/containers/markdown/components/Preview.tsx (1)
app/lib/constants/colors.ts (1)
themes(304-304)
🔇 Additional comments (1)
app/containers/markdown/components/Preview.tsx (1)
2-31: LGTM!Good improvement to use
StyleProp<TextStyle>— the idiomatic React Native type for style props. The conditional spread logic correctly handles both single style objects and arrays, and the default empty array maintains backward compatibility.
tassoevan
had a problem deploying
to
official_android_build
GitHub Actions
Error
tassoevan
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
tassoevan
had a problem deploying
to
official_ios_build
GitHub Actions
Error
tassoevan
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
tassoevan
force-pushed
the
chore/stylistic-changes-1
branch
from
669dd57 to
05072a6
Compare
tassoevan
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
tassoevan
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
tassoevan
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
tassoevan
requested a deployment
to
official_android_build
GitHub Actions
Waiting
tassoevan
requested a deployment
to
experimental_android_build
GitHub Actions
Waiting
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
app/views/RoomView/Banner.tsx (1)
33-34: Drop the lint suppression and pass the style object directly.
MarkdownPreviewnow acceptsStyleProp<TextStyle>, so this single-element array and the eslint disable are no longer needed.style={styles.bannerText}keeps the same behavior and stays aligned with the style cleanup in this PR.♻️ Proposed cleanup
- {/* eslint-disable-next-line react-native/no-single-element-style-arrays */} - <MarkdownPreview msg={text} style={[styles.bannerText]} /> + <MarkdownPreview msg={text} style={styles.bannerText} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/RoomView/Banner.tsx` around lines 33 - 34, The code uses an unnecessary single-element style array and an eslint-disable comment for MarkdownPreview; remove the array and the eslint-disable and pass the style object directly. In RoomView/Banner.tsx update the JSX where <MarkdownPreview msg={text} style={[styles.bannerText]} /> is rendered to use style={styles.bannerText} instead, and delete the preceding eslint-disable-next-line react-native/no-single-element-style-arrays comment so the component receives a StyleProp<TextStyle> as intended.app/containers/markdown/components/Preview.tsx (1)
13-13: Let React Native flattenstyledirectly.The
Array.isArraybranch is extra work here.<Text style={[..., style]}>already handles objects, registered styles, nested arrays, and falsy values, so passingstylethrough directly is simpler and avoids maintaining bespokeStylePropnormalization.♻️ Proposed simplification
interface IMarkdownPreview { msg?: string; numberOfLines?: number; testID?: string; style?: StyleProp<TextStyle>; } -const MarkdownPreview = ({ msg, numberOfLines = 1, style = [], testID }: IMarkdownPreview) => { +const MarkdownPreview = ({ msg, numberOfLines = 1, style, testID }: IMarkdownPreview) => { const { theme } = useTheme(); const formattedText = usePreviewFormatText(msg ?? ''); @@ <Text accessibilityLabel={m} - style={[ - styles.text, - { color: themes[theme].fontDefault, lineHeight: undefined }, - ...(Array.isArray(style) ? style : [style]) - ]} + style={[styles.text, { color: themes[theme].fontDefault, lineHeight: undefined }, style]} numberOfLines={numberOfLines} testID={testID || `markdown-preview-${m}`}>Also applies to: 16-16, 27-30
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/markdown/components/Preview.tsx` at line 13, The Preview component currently manually normalizes the style prop with Array.isArray; remove that custom normalization and pass the style prop directly into the Text/React Native elements (e.g., replace constructs that build style with [..., style] to use style as-is), keeping the prop type as style?: StyleProp<TextStyle>; and apply the same simplification to the other occurrences noted (lines referenced around the Preview render and the blocks at 16 and 27-30) so React Native handles flattening of objects, registered styles, nested arrays, and falsy values natively..eslintrc.js (2)
77-82: Same redundancy:prettierafterplugin:prettier/recommended.The TypeScript override has the same redundant
'prettier'entry.🧹 Proposed fix
extends: [ 'plugin:`@typescript-eslint/recommended`', 'plugin:`@typescript-eslint/eslint-recommended`', '@rocket.chat/eslint-config', - 'plugin:prettier/recommended', - 'prettier' + 'plugin:prettier/recommended' ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.eslintrc.js around lines 77 - 82, Remove the redundant 'prettier' entry that appears after 'plugin:prettier/recommended' in the extends array of .eslintrc.js and also remove the duplicate 'prettier' in the TypeScript override's extends; update the extends lists so each only includes 'plugin:prettier/recommended' (remove the plain 'prettier') to eliminate duplication.
19-25: Redundantprettierin extends array.
plugin:prettier/recommendedalready includeseslint-config-prettier(theprettierconfig), so listing'prettier'separately on line 24 is redundant.🧹 Proposed fix
extends: [ '@rocket.chat/eslint-config', 'plugin:react/recommended', 'plugin:react-hooks/recommended', - 'plugin:prettier/recommended', - 'prettier' + 'plugin:prettier/recommended' ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.eslintrc.js around lines 19 - 25, Remove the redundant 'prettier' entry from the extends array in .eslintrc.js because 'plugin:prettier/recommended' already includes the eslint-config-prettier config; update the extends array (the list containing '@rocket.chat/eslint-config', 'plugin:react/recommended', 'plugin:react-hooks/recommended', 'plugin:prettier/recommended', 'prettier') by deleting the 'prettier' item so only 'plugin:prettier/recommended' remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tsconfig.json`:
- Around line 38-39: The tsconfig enables "noUnusedLocals" and
"noUnusedParameters" but because "checkJs" is disabled, these checks don't apply
to .js files (e.g., app/sagas/state.js and app/sagas/createChannel.js), leaving
JS files uncovered while TS/TSX files are enforced; fix by choosing one
approach: either enable "checkJs" in tsconfig so TypeScript will enforce
unused-variable rules in .js files (update tsconfig to set "checkJs": true) or
enable the ESLint rule "no-unused-vars" for the JavaScript files (adjust ESLint
config to turn on "no-unused-vars" for the app/ directory or add an override for
app/sagas/*), and make sure to remove the global disable that currently
nullifies ESLint's no-unused-vars.
---
Nitpick comments:
In @.eslintrc.js:
- Around line 77-82: Remove the redundant 'prettier' entry that appears after
'plugin:prettier/recommended' in the extends array of .eslintrc.js and also
remove the duplicate 'prettier' in the TypeScript override's extends; update the
extends lists so each only includes 'plugin:prettier/recommended' (remove the
plain 'prettier') to eliminate duplication.
- Around line 19-25: Remove the redundant 'prettier' entry from the extends
array in .eslintrc.js because 'plugin:prettier/recommended' already includes the
eslint-config-prettier config; update the extends array (the list containing
'@rocket.chat/eslint-config', 'plugin:react/recommended',
'plugin:react-hooks/recommended', 'plugin:prettier/recommended', 'prettier') by
deleting the 'prettier' item so only 'plugin:prettier/recommended' remains.
In `@app/containers/markdown/components/Preview.tsx`:
- Line 13: The Preview component currently manually normalizes the style prop
with Array.isArray; remove that custom normalization and pass the style prop
directly into the Text/React Native elements (e.g., replace constructs that
build style with [..., style] to use style as-is), keeping the prop type as
style?: StyleProp<TextStyle>; and apply the same simplification to the other
occurrences noted (lines referenced around the Preview render and the blocks at
16 and 27-30) so React Native handles flattening of objects, registered styles,
nested arrays, and falsy values natively.
In `@app/views/RoomView/Banner.tsx`:
- Around line 33-34: The code uses an unnecessary single-element style array and
an eslint-disable comment for MarkdownPreview; remove the array and the
eslint-disable and pass the style object directly. In RoomView/Banner.tsx update
the JSX where <MarkdownPreview msg={text} style={[styles.bannerText]} /> is
rendered to use style={styles.bannerText} instead, and delete the preceding
eslint-disable-next-line react-native/no-single-element-style-arrays comment so
the component receives a StyleProp<TextStyle> as intended.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 38e48f26-b5f9-4bf1-aac0-b90b8498aee1
⛔ Files ignored due to path filters (5)
app/containers/Avatar/__snapshots__/Avatar.test.tsx.snapis excluded by!**/*.snapapp/containers/markdown/__snapshots__/Markdown.test.tsx.snapis excluded by!**/*.snapapp/containers/message/__snapshots__/Message.test.tsx.snapis excluded by!**/*.snapapp/views/DiscussionsView/__snapshots__/Item.test.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (40)
.eslintignore.eslintrc.js__mocks__/react-native-mmkv.jsapp/containers/ActionSheet/ActionSheet.tsxapp/containers/AudioPlayer/Seek.tsxapp/containers/Avatar/Avatar.stories.tsxapp/containers/Avatar/AvatarContainer.tsxapp/containers/Avatar/AvatarWithEdit.tsxapp/containers/List/ListRadio.tsxapp/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/helpers/getMentionRegexp.test.jsapp/containers/UnreadBadge/getUnreadStyle.test.jsapp/containers/markdown/components/Preview.tsxapp/containers/markdown/components/emoji/Emoji.tsxapp/containers/message/Components/Attachments/Reply.tsxapp/ee/omnichannel/lib/index.tsapp/ee/omnichannel/views/QueueListView.tsxapp/lib/database/utils.test.tsapp/lib/encryption/encryption.tsapp/lib/encryption/helpers/base64-js/index.tsapp/lib/encryption/room.tsapp/lib/helpers/formatText.tsapp/lib/hooks/useShortnameToUnicode/index.tsxapp/lib/methods/search.tsapp/lib/methods/serializeAsciiUrl.test.tsapp/sagas/createChannel.jsapp/sagas/createDiscussion.jsapp/sagas/deepLinking.jsapp/sagas/login.jsapp/sagas/rooms.jsapp/sagas/state.jsapp/views/CreateChannelView/RoomSettings/SwitchItemEncrypted.test.tsxapp/views/DiscussionsView/DiscussionDetails.tsxapp/views/DiscussionsView/Item.tsxapp/views/RoomView/Banner.tsxapp/views/SelectServerView.tsxapp/views/ThreadMessagesView/Item.tsxpackage.jsonscripts/build-icon-set.jstsconfig.json
💤 Files with no reviewable changes (1)
- .eslintignore
🚧 Files skipped from review as they are similar to previous changes (20)
- app/views/SelectServerView.tsx
- app/views/DiscussionsView/Item.tsx
- app/sagas/deepLinking.js
- app/lib/encryption/encryption.ts
- app/containers/ActionSheet/ActionSheet.tsx
- app/lib/database/utils.test.ts
- app/sagas/createDiscussion.js
- app/lib/helpers/formatText.ts
- app/sagas/rooms.js
- app/lib/methods/serializeAsciiUrl.test.ts
- app/containers/MessageComposer/components/Quotes/Quote.tsx
- app/views/CreateChannelView/RoomSettings/SwitchItemEncrypted.test.tsx
- app/views/DiscussionsView/DiscussionDetails.tsx
- app/sagas/login.js
- mocks/react-native-mmkv.js
- app/views/ThreadMessagesView/Item.tsx
- app/containers/UnreadBadge/getUnreadStyle.test.js
- app/containers/markdown/components/emoji/Emoji.tsx
- app/containers/Avatar/AvatarContainer.tsx
- scripts/build-icon-set.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-02-05T13:55:00.974Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:00.974Z
Learning: In this repository, the dependency on react-native-image-crop-picker should reference the RocketChat fork (RocketChat/react-native-image-crop-picker) with explicit commit pins, not the upstream ivpusic/react-native-image-crop-picker. Update package.json dependencies (and any lockfile) to point to the fork URL and a specific commit, ensuring edge-to-edge Android fixes are included. This pattern should apply to all package.json files in the repo that declare this dependency.
Applied to files:
package.json
📚 Learning: 2026-03-05T14:28:17.265Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6997
File: .maestro/tests/room/message-markdown-click.yaml:28-39
Timestamp: 2026-03-05T14:28:17.265Z
Learning: In the Rocket.Chat React Native repository's Maestro test files (.maestro/tests/), regex selectors for partial/contains text matching should use the `'.*keyword.*'` pattern (both leading and trailing `.*`). The pattern `'.*keyword*.'` is incorrect — it parses as zero-or-more of the last character of the keyword followed by one required character, and will fail to match elements where the keyword appears at the end of the element's text. This applies to all Maestro YAML selector fields (text, id) in the codebase.
Applied to files:
app/containers/MessageComposer/helpers/getMentionRegexp.test.js
📚 Learning: 2025-12-17T15:56:22.578Z
Learnt from: OtavioStasiak
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6499
File: app/containers/ServerItem/index.tsx:34-36
Timestamp: 2025-12-17T15:56:22.578Z
Learning: In the Rocket.Chat React Native codebase, for radio button components on iOS, include the selection state ("Selected"/"Unselected") in the accessibilityLabel instead of using accessibilityState={{ checked: hasCheck }}, because iOS VoiceOver has known issues with accessibilityRole="radio" + accessibilityState that prevent correct state announcement.
Applied to files:
app/containers/List/ListRadio.tsx
🔇 Additional comments (12)
app/containers/List/ListRadio.tsx (1)
13-13: Nice lint-safe cleanup.Aliasing
valueto_makes the intent explicit and keeps the public prop shape unchanged. The existing iOS-friendly selected/unselected accessibility label is also preserved.Based on learnings, in the Rocket.Chat React Native codebase radio components on iOS should expose selection state in the accessibility label.
app/ee/omnichannel/lib/index.ts (1)
56-56: Looks good.Dropping the unused binding keeps the module-level initialization intact while removing dead state. The constructor side effects still run when this module is loaded.
app/ee/omnichannel/views/QueueListView.tsx (1)
70-77: LGTM!The underscore prefix on
_datacorrectly signals that the parameter is intentionally unused. This is a standard convention that satisfiesno-unused-varslint rules while preserving the required function signature forgetItemLayout.app/containers/AudioPlayer/Seek.tsx (1)
68-68: LGTM!The underscore prefix convention correctly signals that
_eventis intentionally unused while preserving the required parameter position to accessctx. This aligns with the PR's objective of fixing unused-variable rule inconsistencies.app/lib/methods/search.ts (1)
141-141: Unused executor arg rename is safe.This keeps the cancellation path unchanged while making the intentionally unused parameter explicit.
app/lib/hooks/useShortnameToUnicode/index.tsx (1)
36-36: Regex callback rename looks good.Only the unused capture parameter changed, so the replacement logic remains intact.
app/lib/encryption/helpers/base64-js/index.ts (1)
47-48: Private helper rename is behavior-preserving.The calculation still depends only on
validLenandplaceHoldersLen; this just makes the unused argument explicit.app/lib/encryption/room.ts (1)
503-503: Unusedridrename is fine here.The parameter was already unused in this method, so the underscore prefix improves intent without changing behavior.
.eslintrc.js (3)
101-107: Good: Underscore prefix pattern for unused parameters.
argsIgnorePattern: '^_'aligns with the PR objective to resolve unused-variable rule inconsistencies and follows a common convention for intentionally unused parameters.
14-16: Good: React version auto-detection.Setting
version: 'detect'avoids hardcoding the React version and prevents the "React version not specified" warning.
60-62: The rulesset-state-in-effect,immutability, andrefsare all supported in eslint-plugin-react-hooks v7.0.1.package.json (1)
183-190: All flagged ESLint plugin versions exist on npm and are valid. No installation issues will occur.> Likely an incorrect or invalid review comment.
| "noUnusedLocals": true, /* Report errors on unused locals. */ | ||
| "noUnusedParameters": true, /* Report errors on unused parameters. */ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tsconfig relevant flags =="
sed -n '1,120p' tsconfig.json | rg -n '"allowJs"|"checkJs"|"noUnusedLocals"|"noUnusedParameters"'
echo
echo "== eslint configs mentioning unused-var rules =="
fd -HI '(^|/)(eslint\.config\.(js|cjs|mjs)|\.eslintrc(\.(js|cjs|json|yaml|yml))?)$' -E node_modules -x sh -c '
echo "--- {} ---"
sed -n "1,260p" "{}" | rg -n "no-unused-vars|@typescript-eslint/no-unused-vars|overrides|files"
'
echo
echo "== representative JS files in this PR context =="
printf "%s\n" app/sagas/state.js app/sagas/createChannel.jsRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 912
🏁 Script executed:
sed -n '135,180p' .eslintrc.jsRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 188
JS files have no unused-var enforcement.
With checkJs disabled, TypeScript's noUnusedLocals and noUnusedParameters (lines 38–39) only apply to TS/TSX files. JavaScript files like app/sagas/state.js and app/sagas/createChannel.js rely on ESLint, which has no-unused-vars globally disabled. This creates inconsistent enforcement: TS/TSX files enforce unused variables, but JS files do not. If this PR aims to address unused-variable handling uniformly, consider either enabling checkJs for TypeScript checks on .js files or activating no-unused-vars in ESLint for JavaScript files in the app/ directory.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tsconfig.json` around lines 38 - 39, The tsconfig enables "noUnusedLocals"
and "noUnusedParameters" but because "checkJs" is disabled, these checks don't
apply to .js files (e.g., app/sagas/state.js and app/sagas/createChannel.js),
leaving JS files uncovered while TS/TSX files are enforced; fix by choosing one
approach: either enable "checkJs" in tsconfig so TypeScript will enforce
unused-variable rules in .js files (update tsconfig to set "checkJs": true) or
enable the ESLint rule "no-unused-vars" for the JavaScript files (adjust ESLint
config to turn on "no-unused-vars" for the app/ directory or add an override for
app/sagas/*), and make sure to remove the global disable that currently
nullifies ESLint's no-unused-vars.
| {/* eslint-disable-next-line react-native/no-single-element-style-arrays */} | ||
| {username ? <MarkdownPreview msg={makeThreadName(item)} numberOfLines={2} style={styles.markdown} /> : null} |
There was a problem hiding this comment.
| {/* eslint-disable-next-line react-native/no-single-element-style-arrays */} | |
| {username ? <MarkdownPreview msg={makeThreadName(item)} numberOfLines={2} style={styles.markdown} /> : null} | |
| {/* eslint-disable-next-line react-native/no-single-element-style-arrays */} | |
| {username ? <MarkdownPreview msg={makeThreadName(item)} numberOfLines={2} style={[styles.markdown]} /> : null} |
There was a problem hiding this comment.
I think I've changed MarkdownPreview to handle it:
const MarkdownPreview = ({ msg, numberOfLines = 1, style = [], testID }: IMarkdownPreview) => {
const { theme } = useTheme();
const formattedText = usePreviewFormatText(msg ?? '');
if (!msg) {
return null;
}
const m = formattedText;
return (
<Text
accessibilityLabel={m}
style={[
styles.text,
{ color: themes[theme].fontDefault, lineHeight: undefined },
...(Array.isArray(style) ? style : [style]) // HERE!!!
]}
numberOfLines={numberOfLines}
testID={testID || `markdown-preview-${m}`}>
{m}
</Text>
);
};There was a problem hiding this comment.
Got it, but shouldn't you remove {/* eslint-disable-next-line react-native/no-single-element-style-arrays */} then?
You could also apply this fix to the other places you added the inline lint.
2 participants
Proposed changes
It reviews lint rules to aim consistency.
Issue(s)
Task: NATIVE-49
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
This is one in a sequence of incoming suggested changes.
Summary by CodeRabbit
Chores
Style / UI
Tests