fireproof-storage / fireproof-storage/fireproof

Misleading parameter naming in TokenStrategy.open() method for database name passing

Open
#1,166 1 comment 0 reactions 0 assignees View on GitHub
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.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.