-
Notifications
You must be signed in to change notification settings - Fork 107
fix: code quality and safety improvements #751
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,24 +52,4 @@ export const usePushNotifications = () => { | |
|
|
||
| useEffect(() => { | ||
| registerForPushNotificationsAsync().then(token => setExpoPushToken(token)); | ||
|
|
||
| notificationListener.current = Notifications.addNotificationReceivedListener( | ||
| notification => { | ||
| setNotification(notification); | ||
| }, | ||
| ); | ||
|
|
||
| responseListener.current = Notifications.addNotificationResponseReceivedListener( | ||
| response => { | ||
| logger.debug(response); | ||
| }, | ||
| ); | ||
|
|
||
| return () => { | ||
| Notifications.removeNotificationSubscription(notificationListener.current); | ||
| Notifications.removeNotificationSubscription(responseListener.current); | ||
| }; | ||
| }, []); | ||
|
|
||
| return { expoPushToken, notification }; | ||
| }; | ||
| .catch(err => console.error(err)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf 'Repository files matching usePushNotifications:\n'
fd -a 'usePushNotifications\.js$|usePushNotifications\.jsx$' . || true
printf '\nRelevant source (if present):\n'
if [ -f app/src/lib/usePushNotifications.js ]; then
nl -ba app/src/lib/usePushNotifications.js | sed -n '1,120p'
fi
printf '\nPackage manifests containing expo-notifications:\n'
rg -n --hidden --glob '!node_modules/**' '"expo-notifications"|expo-notifications' package.json pnpm-lock.yaml yarn.lock package-lock.json app/package.json app/package-lock.json 2>/dev/null || true
printf '\nLocal expo-notifications references if available:\n'
for pkg in node_modules expo-notifications node_modules/expo-notifications; do
if [ -e "$pkg" ]; then
echo "FOUND $pkg"
[ -f "$pkg/package.json" ] && sed -n '1,120p' "$pkg/package.json"
rg -n "setNotificationChannelAsync|async" "$pkg/lib" "$pkg" 2>/dev/null | head -200 || true
fi
doneRepository: roshankumar0036singh/Uni-Event Length of output: 342 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf 'Relevant source:\n'
cat -n app/src/lib/usePushNotifications.js | sed -n '1,120p'
printf '\nPackage manifests containing expo-notifications:\n'
rg -n --hidden --glob '!node_modules/**' '"expo-notifications"|expo-notifications' package.json pnpm-lock.yaml yarn.lock package-lock.json app/package.json 2>/dev/null || true
printf '\nLocal expo-notifications package availability:\n'
for dir in node_modules/expo-notifications; do
if [ -d "$dir" ]; then
echo "FOUND $dir"
sed -n '1,140p' "$dir/package.json"
rg -n --hidden "setNotificationChannelAsync|NotificationChannel|exports|ReactNativeAsyncStorage|__esModule" "$dir" 2>/dev/null | head -300 || true
fi
done
printf '\nTop-level dependency files:\n'
for f in package.json app/package.json; do
if [ -f "$f" ]; then
echo "--- $f"
python3 - "$f" <<'PY'
import json, sys
p=json.load(open(sys.argv[1]))
for k,v in p.items():
if k in {"dependencies","devDependencies","peerDependencies"} and isinstance(v, dict):
hits={kk:vv for kk,vv in v.items() if "expo" in kk.lower() or "notification" in kk.lower()}
if hits:
print(k, hits)
PY
fi
doneRepository: roshankumar0036singh/Uni-Event Length of output: 3414 🌐 Web query:
💡 Result: Yes, the function setNotificationChannelAsync in expo-notifications version 0.29.14 returns a Promise [1][2]. According to the official Expo documentation, this method assigns channel configuration to a channel of a specified name (creating it if necessary) [3][4]. The returned Promise resolves to an object of type NotificationChannel that describes the modified channel, or it resolves to null if the platform does not support notification channels [3][4]. As this function is asynchronous, it is standard practice to use the await keyword when calling it [5][6]. Citations:
Handle Android channel-registration failures.
🧰 Tools🪛 Biome (2.5.5)[error] 55-55: Expected a statement but instead found '.catch(err => console.error(err))'. (parse) [error] 55-55: expected (parse) 🤖 Prompt for AI Agents🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/sh
set -eu
npx biome check app/src/lib/usePushNotifications.jsRepository: roshankumar0036singh/Uni-Event Length of output: 2397 🏁 Script executed: #!/bin/sh
set -eu
printf '--- files ---\n'
git ls-files | sed -n '/usePushNotifications/p'
printf '\n--- hook file ---\n'
if [ -f app/src/lib/usePushNotifications.js ]; then
nl -ba app/src/lib/usePushNotifications.js | sed -n '1,120p'
fi
printf '\n--- parser probe ---\n'
node <<'JS'
const fs = require('fs');
const source = fs.readFileSync('app/src/lib/usePushNotifications.js', 'utf8');
try {
new Function(source);
console.log('parse: ok');
} catch (e) {
console.log('parse: failed');
console.log(`${e.name}: ${e.message}`);
for (const p of [e.lineNumber, e.column]) {
console.log(`position: ${p}`);
}
}
JSRepository: roshankumar0036singh/Uni-Event Length of output: 259 🏁 Script executed: #!/bin/sh
set -eu
printf '%s\n' '--- files ---'
git ls-files | sed -n '/usePushNotifications/p'
printf '%s\n' ''
printf '%s\n' '--- hook file ---'
if [ -f app/src/lib/usePushNotifications.js ]; then
nl -ba app/src/lib/usePushNotifications.js | sed -n '1,120p'
else
printf '%s\n' 'missing app/src/lib/usePushNotifications.js'
fi
printf '%s\n' ''
printf '%s\n' '--- parser probe ---'
node <<'JS'
const fs = require('fs');
const source = fs.readFileSync('app/src/lib/usePushNotifications.js', 'utf8');
try {
new Function(source);
console.log('parse: ok');
} catch (e) {
console.log('parse: failed');
console.log(`${e.name}: ${e.message}`);
for (const p of [e.lineNumber, e.column]) {
console.log(`position: ${p}`);
}
}
JSRepository: roshankumar0036singh/Uni-Event Length of output: 458 🏁 Script executed: #!/bin/sh
set -eu
cat -n app/src/lib/usePushNotifications.js | sed -n '1,140p'
printf '%s\n' ''
node --input-type=module <<'JS'
import fs from 'node:fs';
const source = fs.readFileSync('app/src/lib/usePushNotifications.js', 'utf8');
try {
new Function(source);
console.log('parse: ok');
} catch (e) {
console.log('parse: failed');
console.log(`${e.name}: ${e.message}`);
}
JSRepository: roshankumar0036singh/Uni-Event Length of output: 2395 Fix the broken Promise chain. Line 55 uses Proposed fixuseEffect(() => {
registerForPushNotificationsAsync()
.then(token => setExpoPushToken(token))
.catch(err => console.error(err));
}, []);🧰 Tools🪛 Biome (2.5.5)[error] 55-55: Expected a statement but instead found '.catch(err => console.error(err))'. (parse) [error] 55-55: expected (parse) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Keep
response.json()inside each function.The new
const datastatements are not indented under theirtryblocks. They execute at module scope, whereresponseis undefined, anddatais unavailable to the following checks. Indent both statements. Use single quotes for the error text to satisfy Prettier.Proposed fix
Apply the same change in
addToCalendar.Also applies to: 129-130
🧰 Tools
🪛 ESLint
[error] 87-87: Replace
"Request·failed"with'Request·failed'(prettier/prettier)
[error] 88-88: Insert
········(prettier/prettier)
🤖 Prompt for AI Agents
Source: Linters/SAST tools
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Update the successful
fetchmocks withok: true.The success fixtures in
app/src/lib/__tests__/CalendarService.test.jsomitresponse.ok. The new guard therefore evaluates!response.okastrueand both success tests throw before callingjson().Add
ok: trueto the mocked responses atapp/src/lib/__tests__/CalendarService.test.js:23-42andapp/src/lib/__tests__/CalendarService.test.js:63-83.Also applies to: 129-130
🧰 Tools
🪛 ESLint
[error] 87-87: Replace
"Request·failed"with'Request·failed'(prettier/prettier)
[error] 88-88: Insert
········(prettier/prettier)
🤖 Prompt for AI Agents