fireproof-storage / fireproof-storage/fireproof
Misleading parameter naming in TokenStrategy.open() method for database name passing
- Dominant language
- TypeScript
- Stars
- 973
- Forks
- 58
- PR merge metrics
- No merged PRs in 30d
Description
## Issue Description
The `TokenStrategie` interface's `open()` method has a misleading parameter name that creates confusion about what data is being passed to the authentication popup URL.
## Current Problem
In `redirect-strategy.ts` line 94, the code uses:
```typescript
.setParam("local_ledger_name", deviceId);
```
However, the `open()` method signature declares this parameter as `deviceId`:
```typescript
open(sthis: SuperThis, logger: Logger, deviceId: string, opts: ToCloudOpts)
```
But when called from `to-cloud.ts` line 154, it actually passes the database name:
```typescript
this.opts.strategy.open(ledger.sthis, logger, ledger.name, this.opts);
```
## Impact
This works functionally but creates confusion:
1. The parameter is named `deviceId` but contains the database name (e.g., 'kanban-board')
2. Developers reading the code may think device ID is being passed instead of database name
3. This makes it unclear how the local database name gets associated with the cloud ledger
## Proposed Solution
1. **Update TokenStrategie interface** to rename the parameter from `deviceId` to `databaseName` or `ledgerName`
2. **Update all strategy implementations**:
- RedirectStrategy (`redirect-strategy.ts`)
- IframeStrategy (`iframe-strategy.ts`)
- SimpleTokenStrategy (`to-cloud.ts`)
3. **Update callers** in `to-cloud.ts` to use the corrected parameter name
## Files Affected
- `core/types/protocols/cloud/*.ts` (TokenStrategie interface)
- `use-fireproof/redirect-strategy.ts`
- `use-fireproof/iframe-strategy.ts`
- `core/gateways/cloud/to-cloud.ts`
This change would make the code much clearer about how the database name flows through the authentication process for proper cloud sync association.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.