Skip to content
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

fix(web): do not render App on protected routes #1366

Merged
merged 3 commits into from
Jun 21, 2024
Merged

Conversation

imobachgs
Copy link
Contributor

@imobachgs imobachgs commented Jun 21, 2024

When visiting the root route (/), the App component is rendered even if the user is not logged in. Of course, this causes many errors that are not visible in the UI but in the network monitor.

Fixing this problem requires moving the code to handle phase and status changes to a higher place in the hierarchy because, now, App is rendered "too late" to observe status changes.

Failed connection attempts

Captura desde 2024-06-21 15-29-39

@imobachgs imobachgs force-pushed the fix-protected-routes branch 2 times, most recently from db7fe8f to aa8b5dd Compare June 21, 2024 15:26
@@ -27,6 +27,8 @@ import { AppProviders } from "./context/app";
export default function Protected() {
const { isLoggedIn } = useAuth();

if (isLoggedIn === undefined) return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this deserve a bit of comment in code why we do it as it is not obvious especialyl with combination of condition below.

Copy link
Contributor Author

@imobachgs imobachgs Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it is a rather typical pattern in React/JavaScript, but a comment won't hurt.

@imobachgs imobachgs merged commit c9a68ce into master Jun 21, 2024
2 checks passed
@imobachgs imobachgs deleted the fix-protected-routes branch June 21, 2024 19:40
@imobachgs imobachgs added this to the Agama 9 milestone Jun 25, 2024
@imobachgs imobachgs mentioned this pull request Jun 27, 2024
imobachgs added a commit that referenced this pull request Jun 27, 2024
Prepare for releasing Agama 9. It includes the following pull requests:

- #1101
- #1202
- #1228
- #1231
- #1236
- #1238
- #1239
- #1240
- #1242
- #1243
- #1244
- #1245
- #1246
- #1247
- #1248
- #1249
- #1250
- #1251
- #1252
- #1253
- #1254
- #1255
- #1256
- #1257
- #1258
- #1259
- #1260
- #1261
- #1264
- #1265
- #1267
- #1268
- #1269
- #1270
- #1271
- #1272
- #1273
- #1274
- #1279
- #1280
- #1284
- #1285
- #1286
- #1287
- #1288
- #1289
- #1290
- #1291
- #1292
- #1293
- #1294
- #1295
- #1296
- #1298
- #1299
- #1300
- #1301
- #1302
- #1303
- #1304
- #1305
- #1306
- #1307
- #1308
- #1309
- #1310
- #1311
- #1312
- #1313
- #1314
- #1315
- #1316
- #1317
- #1318
- #1319
- #1320
- #1321
- #1322
- #1323
- #1324
- #1325
- #1326
- #1328
- #1329
- #1331
- #1332
- #1334
- #1338
- #1340
- #1341
- #1342
- #1343
- #1344
- #1345
- #1348
- #1349
- #1351
- #1352
- #1353
- #1354
- #1355
- #1356
- #1357
- #1358
- #1359
- #1360
- #1361
- #1362
- #1363
- #1365
- #1366
- #1367
- #1368
- #1371
- #1372
- #1374
- #1375
- #1376
- #1379
- #1380
- #1381
- #1383
- #1384
- #1385
- #1386
- #1387
- #1388
- #1389
- #1391
- #1392
- #1394
- #1395
- #1397
- #1398
- #1399
- #1400
- #1403
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants