Page MenuHomePhabricator
Differential D12828

[lib] Make sure thick threads starts out with startReached
ClosedPublic
Actions

Authored by ashoat on Jul 20 2024, 9:43 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 20, 3:10 PM
Unknown Object (File)
Sun, Apr 20, 9:15 AM
Unknown Object (File)
Sat, Apr 19, 7:01 AM
Unknown Object (File)
Fri, Apr 18, 8:04 AM
Unknown Object (File)
Mar 23 2025, 3:45 AM
Unknown Object (File)
Mar 17 2025, 7:05 PM
Unknown Object (File)
Mar 14 2025, 5:49 AM
Unknown Object (File)
Mar 11 2025, 6:38 AM
View All 98 Files
Subscribers
None

Details

Reviewers
tomek
kamil
marcin
Commits
rCOMMa41c34b166ba: [lib] Make sure thick threads starts out with startReached
Summary

In order to make sure that startReached starts out as true when a thread is created, we set truncationStatus to "exhaustive".

I thought about always using exhaustive, but this conflicts with our plans in the Limit number of displayed DMs project, where we'll want startReached to be false if there are messages in SQLite that are not represented in Redux.

Depends on D12827

Test Plan

I followed the test plan described in this Linear comment. Before this diff, the spinner was visible in the newly-created thick thread. After this diff, it was no longer visible

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Revision Contents
Changeset List

PathSize
lib/
shared/
dm-ops/
2 lines
2 lines

Diff 42628

View Options

lib/shared/dm-ops/create-sidebar-spec.js

// @flow // @flow
import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js'; import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js';
import { isSQLiteSupported } from './utils/db-utils.js'; import { isDesktopSafari, isSQLiteSupported } from './utils/db-utils.js';
import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js'; import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js';
import type { AppState } from '../redux/redux-setup.js'; import type { AppState } from '../redux/redux-setup.js';
import { import {
workerRequestMessageTypes, workerRequestMessageTypes,
type WorkerRequestMessage, type WorkerRequestMessage,
type WorkerResponseMessage, type WorkerResponseMessage,
} from '../types/worker-types.js'; } from '../types/worker-types.js';
Show All 16 Linesclass DatabaseModule {
status: DatabaseStatus; status: DatabaseStatus;
constructor() { constructor() {
const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous
? undefined ? undefined
: preloadedState.currentUserInfo?.id; : preloadedState.currentUserInfo?.id;
const isSupported = isSQLiteSupported(currentLoggedInUserID); const isSupported = isSQLiteSupported(currentLoggedInUserID);
if (!isSupported) { if (!isSupported || isDesktopSafari()) {
tomekUnsubmitted
Not Done
Inline Actions

Can we have a const instead of a function that tells if it is Safari? We can avoid having to detect the browser multiple times, as this fact doesn't change through the lifetime of the app.

tomek: Can we have a const instead of a function that tells if it is Safari? We can avoid having to…
kamilAuthorUnsubmitted
Done
Inline Actions

good point, thanks

kamil: good point, thanks
this.status = databaseStatuses.notSupported; this.status = databaseStatuses.notSupported;
kamilAuthorUnsubmitted
Done
Inline Actions

Naming might be confusing, but at this point database is not supported (we don't have a key to it), it will be supported after main thread will provide encryption key

kamil: Naming might be confusing, but at this point database is not supported (we don't have a key to…
tomekUnsubmitted
Not Done
Inline Actions

Do we change the status later?

tomek: Do we change the status later?
kamilAuthorUnsubmitted
Done
Inline Actions

yes, in initDBForLoggedInUser()

kamil: yes, in `initDBForLoggedInUser()`
} else { } else {
this.init(); this.init();
} }
} }
init(encryptionKey?: ?SubtleCrypto$JsonWebKey) { init(encryptionKey?: ?SubtleCrypto$JsonWebKey) {
this.status = databaseStatuses.initInProgress; this.status = databaseStatuses.initInProgress;
this.worker = new SharedWorker(DATABASE_WORKER_PATH); this.worker = new SharedWorker(DATABASE_WORKER_PATH);
▲ Show 20 Lines • Show All 77 Lines • Show Last 20 Lines
View Options

lib/shared/dm-ops/create-thread-spec.js

// @flow // @flow
import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js'; import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js';
import { isSQLiteSupported } from './utils/db-utils.js'; import { isDesktopSafari, isSQLiteSupported } from './utils/db-utils.js';
import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js'; import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js';
import type { AppState } from '../redux/redux-setup.js'; import type { AppState } from '../redux/redux-setup.js';
import { import {
workerRequestMessageTypes, workerRequestMessageTypes,
type WorkerRequestMessage, type WorkerRequestMessage,
type WorkerResponseMessage, type WorkerResponseMessage,
} from '../types/worker-types.js'; } from '../types/worker-types.js';
Show All 16 Linesclass DatabaseModule {
status: DatabaseStatus; status: DatabaseStatus;
constructor() { constructor() {
const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous
? undefined ? undefined
: preloadedState.currentUserInfo?.id; : preloadedState.currentUserInfo?.id;
const isSupported = isSQLiteSupported(currentLoggedInUserID); const isSupported = isSQLiteSupported(currentLoggedInUserID);
if (!isSupported) { if (!isSupported || isDesktopSafari()) {
tomekUnsubmitted
Not Done
Inline Actions

Can we have a const instead of a function that tells if it is Safari? We can avoid having to detect the browser multiple times, as this fact doesn't change through the lifetime of the app.

tomek: Can we have a const instead of a function that tells if it is Safari? We can avoid having to…
kamilAuthorUnsubmitted
Done
Inline Actions

good point, thanks

kamil: good point, thanks
this.status = databaseStatuses.notSupported; this.status = databaseStatuses.notSupported;
kamilAuthorUnsubmitted
Done
Inline Actions

Naming might be confusing, but at this point database is not supported (we don't have a key to it), it will be supported after main thread will provide encryption key

kamil: Naming might be confusing, but at this point database is not supported (we don't have a key to…
tomekUnsubmitted
Not Done
Inline Actions

Do we change the status later?

tomek: Do we change the status later?
kamilAuthorUnsubmitted
Done
Inline Actions

yes, in initDBForLoggedInUser()

kamil: yes, in `initDBForLoggedInUser()`
} else { } else {
this.init(); this.init();
} }
} }
init(encryptionKey?: ?SubtleCrypto$JsonWebKey) { init(encryptionKey?: ?SubtleCrypto$JsonWebKey) {
this.status = databaseStatuses.initInProgress; this.status = databaseStatuses.initInProgress;
this.worker = new SharedWorker(DATABASE_WORKER_PATH); this.worker = new SharedWorker(DATABASE_WORKER_PATH);
▲ Show 20 Lines • Show All 77 Lines • Show Last 20 Lines