This use of variable 'currentParticle' always evaluates to true.
The best fix is to remove the unnecessary conditional around currentParticleDocPath construction and always compute it directly after the existing early return checks.
In js/desktop/src/features/particles/stream-view.tsx, inside handleToggleReaction (around lines 227–233), replace:
conditional assignment to currentParticleDocPath using currentParticle ? ... : null
subsequent null check if (!currentParticleDocPath) return;
with a direct assignment using currentParticle.id. This keeps functionality unchanged because currentParticle is already guaranteed non-null by the earlier guard.
No imports, new methods, or new dependencies are required.
## Useless conditional
This use of variable 'currentParticle' always evaluates to true.
---
The best fix is to remove the unnecessary conditional around <code>currentParticleDocPath</code> construction and always compute it directly after the existing early return checks.</p>
<p>In <code>js/desktop/src/features/particles/stream-view.tsx</code>, inside <code>handleToggleReaction</code> (around lines 227–233), replace:</p>
<ul>
<li>conditional assignment to <code>currentParticleDocPath</code> using <code>currentParticle ? ... : null</code></li>
<li>subsequent null check <code>if (!currentParticleDocPath) return;</code></li>
</ul>
<p>with a direct assignment using <code>currentParticle.id</code>. This keeps functionality unchanged because <code>currentParticle</code> is already guaranteed non-null by the earlier guard.</p>
<p>No imports, new methods, or new dependencies are required.
github-code-quality[bot]
(Migrated from github.com)
reviewed 2026-06-01 21:45:17 +00:00
Remove only the unnecessary 'worklet'; directive in the panUp.onEnd callback (around line 338 in js/mobile/src/features/stream-view/StreamView.tsx).
Best fix without functional change:
Keep the gesture logic intact.
Keep runOnJS(openReactions)() intact.
Delete the single directive statement from that callback body.
No imports, methods, or new definitions are needed.
## Unknown directive
Unknown directive: 'worklet'.
---
Remove only the unnecessary <code>'worklet';</code> directive in the <code>panUp</code> <code>.onEnd</code> callback (around line 338 in <code>js/mobile/src/features/stream-view/StreamView.tsx</code>).</p>
<p>Best fix without functional change:</p>
<ul>
<li>Keep the gesture logic intact.</li>
<li>Keep <code>runOnJS(openReactions)()</code> intact.</li>
<li>Delete the single directive statement from that callback body.</li>
</ul>
<p>No imports, methods, or new definitions are needed.
This PR contains 250 files, which is 100 over the limit of 150.
To get a review, narrow the scope:
• coderabbit review --type committed # exclude uncommitted changes
• coderabbit review --dir # limit to a subdirectory
• coderabbit review --base # compare against a closer base
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d99a9a3-9b51-4a83-83bb-5f6859747ca2
📥 Commits
Reviewing files that changed from the base of the PR and between 2fe562ce2b and 89b828a576.
⛔ Files ignored due to path filters (2)
js/desktop/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
js/mobile/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
For best results, initiate chat on the files or code changes.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# List the structure of js/mobile directory
tree js/mobile -L 4 2>/dev/null || find js/mobile -type f | head -80
echo"---"# Check what files were changed in this PR within js/mobile
gh pr view 230 --repo flowy-live/llink --json files --jq '.files[].path'| grep "^js/mobile"||echo"No changed files in js/mobile found via API"echo"---"# Show the diff for js/mobile in this PR
gh pr diff 230 --repo flowy-live/llink -- js/mobile 2>/dev/null | head -300
@talksik — Note that PR #230 only touches js/desktop; js/mobile has no changed files in this diff. Here's a review of the current state of js/mobile:
Review: js/mobile
🔴 High — Sentry never initialized (src/App.tsx + src/lib/errors.ts)
src/lib/errors.ts exports installErrorSinks() and its own comment says it must be called from App.tsx, but App.tsx never imports or invokes it. This means captureSink and breadcrumbSink remain null for the entire app lifetime — Sentry is configured in src/config/env.ts but completely dead.
// src/App.tsx — add Sentry wiring, e.g.:
import{installErrorSinks}from'`@/lib/errors`';import*asSentryfrom'`@sentry/react-native`';// or whichever SDK you're using
Sentry.init({dsn: appConfig.sentryDsn,environment: appEnv});installErrorSinks({capture:(err,ctx)=>Sentry.captureException(err,{extra: ctx}),breadcrumb:(err,ctx)=>Sentry.addBreadcrumb({message: String(err),data: ctx}),});
🟡 Medium — format script doesn't cover root-level config files
This misses app.config.ts, babel.config.js, metro.config.js, tailwind.config.js, eslint.config.js, and eas.json. All of these use double-quoted strings, which violates the .prettierrc setting "singleQuote": true. Suggested fix:
// current
const{defineConfig}=require("eslint/config");// should be (after running prettier on it)
const{defineConfig}=require('eslint/config');
🟡 Medium — app.config.ts: stray blank line + missing trailing comma
app.config.ts (not covered by the current format script) has a blank line in the extra block and a missing trailing comma inside eas:
// current
extra:{eas:{projectId:"e902f7e5-e514-42bd-9591-75738e3494ed"// ← missing trailing comma
},// ← stray blank line
appEnv: APP_ENV,},// after prettier
extra:{eas:{projectId:'e902f7e5-e514-42bd-9591-75738e3494ed',},appEnv: APP_ENV,},
Both are resolved automatically once app.config.ts is included in the format glob above.
🔵 Low — Test-only export in production bundle (src/lib/push-notifications.ts)
_resetPushNotificationsModule() is documented as "Test-only" but is exported unconditionally and will be included in the production bundle. Consider guarding it:
// Only export in non-production environments
exportconst_resetPushNotificationsModule=process.env.NODE_ENV!=='production'?():void=>{tokenListenerSubscription?.remove();tokenListenerSubscription=null;configured=false;}:undefined;
Or move it to a test helper file that's excluded from the app bundle.
🔵 Low — Hardcoded colour in SignInScreen.tsx
// current
placeholderTextColor="`#878787`"// prefer theme token (matches tailwind `ring` / `muted-foreground`)
import{colors}from'`@/lib/colors`';// or however you expose theme tokens
placeholderTextColor={colors.mutedForeground}
✅ What looks good
ESLint config (eslint.config.js) — expo flat config + eslint-config-prettier layering is correct; @typescript-eslint/no-unused-vars with ^_ ignore patterns is a good default.
.prettierrc settings are sensible and consistent with js/desktop.
src/lib/errors.ts error-classification and user-message logic is thorough.
src/stores/auth-store.ts sign-out ordering (stop sync → unregister token → call API → clear local) is correct.
src/lib/query-client.ts 401 → invalidateSession centralisation is clean.
Firebase long-polling opt-in (experimentalAutoDetectLongPolling: true) is the right call for React Native.
Push notification token-sync architecture (only active while authenticated) is well thought out.
<!-- This is an auto-generated reply by CodeRabbit -->
> [!TIP]
> For best results, initiate chat on the files or code changes.
<details>
<summary>🧩 Analysis chain</summary>
🏁 Script executed:
```shell
#!/bin/bash
# List the structure of js/mobile directory
tree js/mobile -L 4 2>/dev/null || find js/mobile -type f | head -80
echo "---"
# Check what files were changed in this PR within js/mobile
gh pr view 230 --repo flowy-live/llink --json files --jq '.files[].path' | grep "^js/mobile" || echo "No changed files in js/mobile found via API"
echo "---"
# Show the diff for js/mobile in this PR
gh pr diff 230 --repo flowy-live/llink -- js/mobile 2>/dev/null | head -300
```
Length of output: 3517
---
🏁 Script executed:
```shell
#!/bin/bash
# Check all changed files in PR `#230`
gh pr view 230 --repo flowy-live/llink --json files --jq '.files[].path'
echo "=== PR description ==="
gh pr view 230 --repo flowy-live/llink --json body --jq '.body'
echo "=== eslint config in js/mobile ==="
cat js/mobile/eslint.config.js
echo "=== package.json scripts in js/mobile ==="
cat js/mobile/package.json | python3 -c "import json,sys; d=json.load(sys.stdin); print(json.dumps({'scripts': d.get('scripts',{}), 'devDependencies': d.get('devDependencies',{})}, indent=2))"
```
Length of output: 6811
---
🏁 Script executed:
```shell
#!/bin/bash
echo "=== Check for prettier config in js/mobile ==="
find js/mobile -name ".prettierrc*" -o -name "prettier.config.*" 2>/dev/null
echo "=== tsconfig.json ==="
cat js/mobile/tsconfig.json
echo "=== babel.config.js ==="
cat js/mobile/babel.config.js
echo "=== metro.config.js ==="
cat js/mobile/metro.config.js
echo "=== tailwind.config.js ==="
cat js/mobile/tailwind.config.js
echo "=== app.config.ts ==="
cat js/mobile/app.config.ts
echo "=== index.ts ==="
cat js/mobile/index.ts
echo "=== src/App.tsx ==="
cat js/mobile/src/App.tsx
echo "=== src/api/client.ts ==="
cat js/mobile/src/api/client.ts
echo "=== src/api/types.ts ==="
cat js/mobile/src/api/types.ts
echo "=== src/config/env.ts ==="
cat js/mobile/src/config/env.ts
echo "=== src/firebase.ts ==="
cat js/mobile/src/firebase.ts
```
Length of output: 29333
---
🏁 Script executed:
```shell
#!/bin/bash
echo "=== .prettierrc ==="
cat js/mobile/.prettierrc
echo "=== eas.json ==="
cat js/mobile/eas.json
echo "=== src/lib/errors.ts ==="
cat js/mobile/src/lib/errors.ts
echo "=== src/stores/auth-store.ts ==="
cat js/mobile/src/stores/auth-store.ts
echo "=== src/navigation/RootNavigator.tsx ==="
cat js/mobile/src/navigation/RootNavigator.tsx
echo "=== src/lib/query-client.ts ==="
cat js/mobile/src/lib/query-client.ts
echo "=== src/lib/push-notifications.ts ==="
cat js/mobile/src/lib/push-notifications.ts
echo "=== src/features/auth/SignInScreen.tsx ==="
cat js/mobile/src/features/auth/SignInScreen.tsx
echo "=== nativewind-env.d.ts ==="
cat js/mobile/nativewind-env.d.ts
echo "=== global.css ==="
cat js/mobile/global.css
```
Length of output: 25380
---
</details>
---
`@talksik` — Note that PR `#230` only touches `js/desktop`; `js/mobile` has no changed files in this diff. Here's a review of the current state of `js/mobile`:
---
## Review: `js/mobile`
### 🔴 High — Sentry never initialized (`src/App.tsx` + `src/lib/errors.ts`)
`src/lib/errors.ts` exports `installErrorSinks()` and its own comment says it must be called from `App.tsx`, but `App.tsx` never imports or invokes it. This means `captureSink` and `breadcrumbSink` remain `null` for the entire app lifetime — Sentry is configured in `src/config/env.ts` but completely dead.
```tsx
// src/App.tsx — add Sentry wiring, e.g.:
import { installErrorSinks } from '`@/lib/errors`';
import * as Sentry from '`@sentry/react-native`'; // or whichever SDK you're using
Sentry.init({ dsn: appConfig.sentryDsn, environment: appEnv });
installErrorSinks({
capture: (err, ctx) => Sentry.captureException(err, { extra: ctx }),
breadcrumb: (err, ctx) => Sentry.addBreadcrumb({ message: String(err), data: ctx }),
});
```
---
### 🟡 Medium — `format` script doesn't cover root-level config files
`package.json`:
```json
"format": "prettier --write \"src/**/*.{ts,tsx,js,jsx,json,css,md}\""
```
This misses `app.config.ts`, `babel.config.js`, `metro.config.js`, `tailwind.config.js`, `eslint.config.js`, and `eas.json`. All of these use double-quoted strings, which violates the `.prettierrc` setting `"singleQuote": true`. Suggested fix:
```json
"format": "prettier --write \"src/**/*.{ts,tsx,js,jsx,json,css,md}\" \"*.{ts,tsx,js,cjs,json}\"",
"format:check": "prettier --check \"src/**/*.{ts,tsx,js,jsx,json,css,md}\" \"*.{ts,tsx,js,cjs,json}\""
```
Similarly, `eslint.config.js` itself uses double quotes:
```js
// current
const { defineConfig } = require("eslint/config");
// should be (after running prettier on it)
const { defineConfig } = require('eslint/config');
```
---
### 🟡 Medium — `app.config.ts`: stray blank line + missing trailing comma
`app.config.ts` (not covered by the current format script) has a blank line in the `extra` block and a missing trailing comma inside `eas`:
```ts
// current
extra: {
eas: {
projectId: "e902f7e5-e514-42bd-9591-75738e3494ed" // ← missing trailing comma
},
// ← stray blank line
appEnv: APP_ENV,
},
// after prettier
extra: {
eas: {
projectId: 'e902f7e5-e514-42bd-9591-75738e3494ed',
},
appEnv: APP_ENV,
},
```
Both are resolved automatically once `app.config.ts` is included in the format glob above.
---
### 🔵 Low — Test-only export in production bundle (`src/lib/push-notifications.ts`)
`_resetPushNotificationsModule()` is documented as "Test-only" but is exported unconditionally and will be included in the production bundle. Consider guarding it:
```ts
// Only export in non-production environments
export const _resetPushNotificationsModule =
process.env.NODE_ENV !== 'production'
? (): void => {
tokenListenerSubscription?.remove();
tokenListenerSubscription = null;
configured = false;
}
: undefined;
```
Or move it to a test helper file that's excluded from the app bundle.
---
### 🔵 Low — Hardcoded colour in `SignInScreen.tsx`
```tsx
// current
placeholderTextColor="`#878787`"
// prefer theme token (matches tailwind `ring` / `muted-foreground`)
import { colors } from '`@/lib/colors`'; // or however you expose theme tokens
placeholderTextColor={colors.mutedForeground}
```
---
### ✅ What looks good
- ESLint config (`eslint.config.js`) — expo flat config + `eslint-config-prettier` layering is correct; `@typescript-eslint/no-unused-vars` with `^_` ignore patterns is a good default.
- `.prettierrc` settings are sensible and consistent with `js/desktop`.
- `src/lib/errors.ts` error-classification and user-message logic is thorough.
- `src/stores/auth-store.ts` sign-out ordering (stop sync → unregister token → call API → clear local) is correct.
- `src/lib/query-client.ts` 401 → `invalidateSession` centralisation is clean.
- Firebase long-polling opt-in (`experimentalAutoDetectLongPolling: true`) is the right call for React Native.
- Push notification token-sync architecture (only active while authenticated) is well thought out.
talksik
(Migrated from github.com)
reviewed 2026-06-02 00:50:02 +00:00
For best results, initiate chat on the files or code changes.
@talksik — The full js/mobile review was already posted just above in this thread (my previous reply). Here's a quick summary of the findings in case it's easier to scan:
Severity
Finding
🔴 High
Sentry never initialized — installErrorSinks() is never called from App.tsx, so all error/breadcrumb sinks remain null
🟡 Medium
format script glob misses root-level config files (app.config.ts, babel.config.js, metro.config.js, etc.) — they violate .prettierrc's singleQuote: true
🟡 Medium
app.config.ts has a missing trailing comma + stray blank line (auto-fixed once included in the format glob)
🔵 Low
_resetPushNotificationsModule() is a test-only export shipped in the production bundle
🔵 Low
Hardcoded colour #878787 in SignInScreen.tsx — should use a theme token
The full code snippets and suggested fixes are in my previous comment. Let me know if you'd like me to dive deeper into any specific area or file!
<!-- This is an auto-generated reply by CodeRabbit -->
> [!TIP]
> For best results, initiate chat on the files or code changes.
`@talksik` — The full `js/mobile` review was already posted just above in this thread (my previous reply). Here's a quick summary of the findings in case it's easier to scan:
| Severity | Finding |
|----------|---------|
| 🔴 High | Sentry never initialized — `installErrorSinks()` is never called from `App.tsx`, so all error/breadcrumb sinks remain `null` |
| 🟡 Medium | `format` script glob misses root-level config files (`app.config.ts`, `babel.config.js`, `metro.config.js`, etc.) — they violate `.prettierrc`'s `singleQuote: true` |
| 🟡 Medium | `app.config.ts` has a missing trailing comma + stray blank line (auto-fixed once included in the format glob) |
| 🔵 Low | `_resetPushNotificationsModule()` is a test-only export shipped in the production bundle |
| 🔵 Low | Hardcoded colour `#878787` in `SignInScreen.tsx` — should use a theme token |
The full code snippets and suggested fixes are in my previous comment. Let me know if you'd like me to dive deeper into any specific area or file!
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #145
@@ -422,14 +477,17 @@ function StreamViewInner({ path, streamParticle }: StreamViewProps) {)}Useless conditional
This use of variable 'currentParticle' always evaluates to true.
The best fix is to remove the unnecessary conditional around
currentParticleDocPathconstruction and always compute it directly after the existing early return checks.In
js/desktop/src/features/particles/stream-view.tsx, insidehandleToggleReaction(around lines 227–233), replace:currentParticleDocPathusingcurrentParticle ? ... : nullif (!currentParticleDocPath) return;with a direct assignment using
currentParticle.id. This keeps functionality unchanged becausecurrentParticleis already guaranteed non-null by the earlier guard.No imports, new methods, or new dependencies are required.
@@ -164,3 +170,3 @@{...wrapperProps}style={{ flex: 1, justifyContent: "flex-end" }}style={{ flex: 1, justifyContent: 'flex-end' }}pointerEvents="box-none"Unknown directive
Unknown directive: 'worklet'.
@@ -172,3 +178,3 @@backgroundColor: "#1c1c1c",backgroundColor: '#1c1c1c',borderTopLeftRadius: 22,borderTopRightRadius: 22,Unknown directive
Unknown directive: 'worklet'.
Unknown directive
Unknown directive: 'worklet'.
@@ -190,180 +195,184 @@ export function ReactionSheet({onRequestClose={dismiss}Unknown directive
Unknown directive: 'worklet'.
Unknown directive
Unknown directive: 'worklet'.
@@ -613,3 +607,3 @@videoFit={videoFit}onToggleVideoFit={() =>setVideoFit((v) => (v === "cover" ? "contain" : "cover"))setVideoFit((v) => (v === 'cover' ? 'contain' : 'cover'))Unknown directive
Unknown directive: 'worklet'.
@@ -647,3 +641,3 @@bottom: insets.bottom + COMPOSE_DOCK_HEIGHT + 40,justifyContent: "center",justifyContent: 'center',}}Unknown directive
Unknown directive: 'worklet'.
Remove only the unnecessary
'worklet';directive in thepanUp.onEndcallback (around line 338 injs/mobile/src/features/stream-view/StreamView.tsx).Best fix without functional change:
runOnJS(openReactions)()intact.No imports, methods, or new definitions are needed.
Unknown directive
Unknown directive: 'worklet'.
@@ -690,3 +683,4 @@streamStatus={streamParticle.status ?? 'open'}isCreator={isCreator}canEditParticle={canEditCurrentParticle}canDeleteParticle={canDeleteCurrentParticle}Unknown directive
Unknown directive: 'worklet'.
✨ Finishing Touches
🧪 Generate unit tests (beta)
resolve-lint-issuesComment
@coderabbitai helpto get the list of available commands and usage tips.coderabbit review --dir js/mobile
🧩 Analysis chain
🏁 Script executed:
Length of output: 3517
🏁 Script executed:
Length of output: 6811
🏁 Script executed:
Length of output: 29333
🏁 Script executed:
Length of output: 25380
@talksik— Note that PR#230only touchesjs/desktop;js/mobilehas no changed files in this diff. Here's a review of the current state ofjs/mobile:Review:
js/mobile🔴 High — Sentry never initialized (
src/App.tsx+src/lib/errors.ts)src/lib/errors.tsexportsinstallErrorSinks()and its own comment says it must be called fromApp.tsx, butApp.tsxnever imports or invokes it. This meanscaptureSinkandbreadcrumbSinkremainnullfor the entire app lifetime — Sentry is configured insrc/config/env.tsbut completely dead.🟡 Medium —
formatscript doesn't cover root-level config filespackage.json:This misses
app.config.ts,babel.config.js,metro.config.js,tailwind.config.js,eslint.config.js, andeas.json. All of these use double-quoted strings, which violates the.prettierrcsetting"singleQuote": true. Suggested fix:Similarly,
eslint.config.jsitself uses double quotes:🟡 Medium —
app.config.ts: stray blank line + missing trailing commaapp.config.ts(not covered by the current format script) has a blank line in theextrablock and a missing trailing comma insideeas:Both are resolved automatically once
app.config.tsis included in the format glob above.🔵 Low — Test-only export in production bundle (
src/lib/push-notifications.ts)_resetPushNotificationsModule()is documented as "Test-only" but is exported unconditionally and will be included in the production bundle. Consider guarding it:Or move it to a test helper file that's excluded from the app bundle.
🔵 Low — Hardcoded colour in
SignInScreen.tsx✅ What looks good
eslint.config.js) — expo flat config +eslint-config-prettierlayering is correct;@typescript-eslint/no-unused-varswith^_ignore patterns is a good default..prettierrcsettings are sensible and consistent withjs/desktop.src/lib/errors.tserror-classification and user-message logic is thorough.src/stores/auth-store.tssign-out ordering (stop sync → unregister token → call API → clear local) is correct.src/lib/query-client.ts401 →invalidateSessioncentralisation is clean.experimentalAutoDetectLongPolling: true) is the right call for React Native.coderabbit autofix to simplify the comment
coderabbit review --dir ./js/mobile
@coderabbit review --dir ./js/mobile
@talksik— The fulljs/mobilereview was already posted just above in this thread (my previous reply). Here's a quick summary of the findings in case it's easier to scan:installErrorSinks()is never called fromApp.tsx, so all error/breadcrumb sinks remainnullformatscript glob misses root-level config files (app.config.ts,babel.config.js,metro.config.js, etc.) — they violate.prettierrc'ssingleQuote: trueapp.config.tshas a missing trailing comma + stray blank line (auto-fixed once included in the format glob)_resetPushNotificationsModule()is a test-only export shipped in the production bundle#878787inSignInScreen.tsx— should use a theme tokenThe full code snippets and suggested fixes are in my previous comment. Let me know if you'd like me to dive deeper into any specific area or file!