-
Notifications
You must be signed in to change notification settings - Fork 10
feat(mrt-utilities): add development data-store with dist export #384
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
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 |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@salesforce/mrt-utilities': patch | ||
| --- | ||
|
|
||
| Add a development-mode pseudo data-store implementation for `@salesforce/mrt-utilities/data-store` with environment-variable-backed defaults, while preserving the existing public API and production behavior. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| # Development Data Store Plan (`@salesforce/mrt-utilities`) | ||
|
|
||
| ## Goal | ||
|
|
||
| Add a pseudo local data-store implementation for development mode so local runtimes do not fail when DynamoDB-backed MRT data store is unavailable. | ||
|
|
||
| This behavior should be activated through the existing package export condition: | ||
|
|
||
| - `@salesforce/mrt-utilities/data-store` + `--conditions development` | ||
|
|
||
| ## Key Requirement | ||
|
|
||
| The pseudo local implementation must read default entry values from environment variables (as implied by the provided prototype): | ||
|
|
||
| - `SFNEXT_DATA_STORE_DEFAULTS` (JSON object map of key -> value object) | ||
| - `SFNEXT_DATA_STORE_WARN_ON_MISSING` (`"false"` disables warnings; default is warning enabled) | ||
|
|
||
| ## Current State | ||
|
|
||
| - `./data-store` already has a `development` export condition in `package.json`, but it currently points to `src`. | ||
| - Desired update: point `development` to a built `dist` pseudo-local data-store output. | ||
| - Current `DataStore` implementation is DynamoDB-based and throws `DataStoreUnavailableError` when required MRT environment variables are missing. | ||
| - Existing tests primarily validate production/DynamoDB behavior. | ||
|
|
||
| ## Proposed Design | ||
|
|
||
| ## 1) Split data-store implementations | ||
|
|
||
| Create two implementation modules: | ||
|
|
||
| - **Production implementation** (existing behavior) | ||
| - DynamoDB-backed | ||
| - Preserves existing errors: | ||
| - `DataStoreUnavailableError` | ||
| - `DataStoreNotFoundError` | ||
| - `DataStoreServiceError` | ||
| - **Development implementation** (new behavior) | ||
| - No AWS dependency | ||
| - Uses local defaults from env var JSON | ||
| - Warns once per missing key (configurable) | ||
| - Uses strict parity with production semantics by default for missing keys (throws not-found) | ||
| - Optional lenient mode can be introduced as explicit opt-in for `{}` fallback during local experimentation | ||
|
|
||
| ## 2) Preserve stable public API | ||
|
|
||
| Keep consumer import surface unchanged: | ||
|
|
||
| - `DataStore.getDataStore()` | ||
| - `DataStore#isDataStoreAvailable()` | ||
| - `DataStore#getEntry(key)` | ||
| - Existing error classes remain exported | ||
|
|
||
| This allows existing projects to adopt dev behavior without refactoring imports. | ||
|
|
||
| ## 3) Route development exports | ||
|
|
||
| Use conditional exports to load the development implementation for local dev from built artifacts: | ||
|
|
||
| - `development` -> built dev pseudo-local data-store module in `dist` | ||
| - `import` / `require` -> production built outputs in `dist` | ||
|
|
||
| ## 4) Environment variable behavior in dev store | ||
|
|
||
| ### `SFNEXT_DATA_STORE_DEFAULTS` | ||
|
|
||
| - Parse as JSON object. | ||
| - Expected shape: | ||
| - `{ "<entry-key>": { ...objectValue } }` | ||
| - On invalid JSON: | ||
| - fall back to empty defaults | ||
| - warn once with clear message | ||
|
|
||
| ### `SFNEXT_DATA_STORE_WARN_ON_MISSING` | ||
|
|
||
| - If unset: warnings enabled | ||
| - If set to `"false"` (case-insensitive): disable missing-key warnings | ||
| - Any other value: warnings enabled | ||
|
|
||
| ### Missing key semantics (dev mode) | ||
|
|
||
| - If key exists in parsed defaults and value is object: return that value. | ||
| - If key missing or invalid value type: | ||
| - by default, throw `DataStoreNotFoundError` (production parity) | ||
| - optionally warn once for that key before throwing | ||
| - optional future opt-in lenient mode may return `{}` instead (must be off by default) | ||
|
|
||
| ## 5) Tests | ||
|
|
||
| Add/adjust tests to cover both modes: | ||
|
|
||
| - **Production tests** | ||
| - Keep current behavior assertions unchanged. | ||
| - **Development tests** | ||
| - Reads defaults from `SFNEXT_DATA_STORE_DEFAULTS` | ||
| - Throws `DataStoreNotFoundError` when key is absent (default behavior) | ||
| - Warns once per missing key when warnings enabled | ||
| - Does not warn when `SFNEXT_DATA_STORE_WARN_ON_MISSING=false` | ||
| - Handles invalid JSON safely | ||
| - (If lenient mode is added) returns `{}` only when explicitly enabled | ||
|
|
||
| ## 6) Documentation updates | ||
|
|
||
| Update `packages/mrt-utilities/README.md` (or docs page if preferred) with: | ||
|
|
||
| - How to enable dev behavior (`node --conditions development`) | ||
| - Env var configuration examples for default data-store values | ||
| - Differences between dev and production data-store semantics | ||
|
|
||
| ## Implementation Steps | ||
|
|
||
| 1. Add a new dev data-store module in `src/data-store/`. | ||
| 2. Move/keep current DynamoDB implementation as production module. | ||
| 3. Ensure build output emits both implementations to `dist` (esm/cjs + types). | ||
| 4. Update `package.json` exports so `development` resolves to the built dev pseudo-local module in `dist` (not `src`), while `import`/`require` continue resolving to production built outputs. | ||
| 5. Add development-focused tests. | ||
| 6. Run validation: | ||
| - `pnpm --filter @salesforce/mrt-utilities run test:agent` | ||
| - `pnpm --filter @salesforce/mrt-utilities run lint:agent` | ||
| - `pnpm --filter @salesforce/mrt-utilities run typecheck:agent` | ||
| 7. Add a changeset for `@salesforce/mrt-utilities` if this is considered user-facing behavior. | ||
|
|
||
| ## Risks / Notes | ||
|
|
||
| - Strict production parity in dev is the default to avoid masking missing-key issues. | ||
| - Any lenient `{}` fallback behavior must be explicit opt-in and clearly documented. | ||
| - Existing export stripping behavior is already understood and is not changed by this plan. | ||
|
|
||
| ## Acceptance Criteria | ||
|
|
||
| - Local development using `--conditions development` no longer fails due to missing DynamoDB/MRT runtime vars. | ||
| - Dev data-store entries are sourced from `SFNEXT_DATA_STORE_DEFAULTS`. | ||
| - Missing-key behavior is predictable and configurable via `SFNEXT_DATA_STORE_WARN_ON_MISSING`. | ||
| - Production behavior and API remain backward-compatible. | ||
| - No breaking public interface changes: existing import paths, exported symbols, and type surface for `@salesforce/mrt-utilities` and `@salesforce/mrt-utilities/data-store` remain intact (except correcting the `development` export target to built `dist` output). | ||
| - Default dev missing-key semantics match production (`DataStoreNotFoundError`), with no implicit `{}` fallback. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,6 +52,16 @@ | |
| } | ||
| }, | ||
| "./data-store": { | ||
| "dev-data-store": { | ||
| "import": { | ||
| "types": "./dist/esm/data-store/development.d.ts", | ||
| "default": "./dist/esm/data-store/development.js" | ||
| }, | ||
| "require": { | ||
| "types": "./dist/cjs/data-store/development.d.ts", | ||
| "default": "./dist/cjs/data-store/development.js" | ||
| } | ||
| }, | ||
| "development": "./src/data-store/index.ts", | ||
|
Comment on lines
+55
to
65
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. If I understand correctly, this line: "development": "./src/data-store/index.ts",is intended for internal monorepo development currently — but it also has implications for downstream consumers, right? Why not lean into that and optimize the development condition for downstream consumers instead? Non-blocking suggestion: could we conditionally rewrite the exports during the Concretely, during build we'd replace: "development": "./src/data-store/index.ts",with: "development": {
"import": {
"types": "./dist/esm/data-store/development.d.ts",
"default": "./dist/esm/data-store/development.js"
},
"require": {
"types": "./dist/cjs/data-store/development.d.ts",
"default": "./dist/cjs/data-store/development.js"
}
}Happy to discuss if there's context I'm missing.
Collaborator
Author
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. That is a subtlety that I touch upon in our conversation earlier. Currently we have a pattern, condition=development means we point at the source. I didn't want to break that convention by pointing at a different built module. I actually had it this way but I didn't want to semantically overload the word "development" in this case. Long story short, I don't want to rock the boat too much. Since vite probably uses that development condition I don't want it to mean one thing for one file and another for a different one.
Collaborator
Author
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. Lets broach this subject with @clavery when he is back to work |
||
| "import": { | ||
| "types": "./dist/esm/data-store/index.d.ts", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| /* | ||
| * Copyright (c) 2025, Salesforce, Inc. | ||
| * SPDX-License-Identifier: Apache-2 | ||
| * For full license text, see the license.txt file in the repo root or http://www.apache.org/licenses/LICENSE-2.0 | ||
| */ | ||
|
|
||
| import {DataStoreNotFoundError} from './errors.js'; | ||
|
|
||
| export {DataStoreNotFoundError, DataStoreServiceError, DataStoreUnavailableError} from './errors.js'; | ||
|
|
||
| /** | ||
| * Development-only pseudo data store backed by environment variables. | ||
| * | ||
| * This class mirrors the public DataStore API while avoiding DynamoDB access. | ||
| */ | ||
| export class DataStore { | ||
| private defaults: Record<string, Record<string, unknown>>; | ||
|
|
||
| private warnOnMissing: boolean; | ||
|
|
||
| private warnedKeys: Set<string>; | ||
|
|
||
| private static _instance: DataStore | null = null; | ||
|
|
||
| private constructor() { | ||
| this.defaults = readDefaultsFromEnv(); | ||
| this.warnOnMissing = readWarnOnMissingFromEnv(); | ||
| this.warnedKeys = new Set<string>(); | ||
| } | ||
|
|
||
| /** | ||
| * Get or create the singleton DataStore instance. | ||
| * | ||
| * @returns The singleton DataStore instance | ||
| */ | ||
| static getDataStore(): DataStore { | ||
| if (!DataStore._instance) { | ||
| DataStore._instance = new DataStore(); | ||
| } | ||
|
|
||
| return DataStore._instance; | ||
| } | ||
|
|
||
| /** | ||
| * Whether the data store can be used in the current environment. | ||
| * | ||
| * The development pseudo store is always available when loaded. | ||
| * | ||
| * @returns true | ||
| */ | ||
| isDataStoreAvailable(): boolean { | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Fetch an entry from the pseudo data store. | ||
| * | ||
| * @param key The data store entry's key | ||
| * @returns An object containing the entry's key and value | ||
| * @throws {DataStoreNotFoundError} An entry with the given key cannot be found | ||
| */ | ||
| async getEntry(key: string): Promise<Record<string, unknown> | undefined> { | ||
| const value = this.defaults[key]; | ||
| if (value && typeof value === 'object') { | ||
| return {key, value}; | ||
| } | ||
|
|
||
| if (this.warnOnMissing && !this.warnedKeys.has(key)) { | ||
| this.warnedKeys.add(key); | ||
| console.warn(`Local data-store provider did not find '${key}'.`); | ||
| } | ||
|
|
||
| throw new DataStoreNotFoundError(`Data store entry '${key}' not found.`); | ||
| } | ||
| } | ||
|
|
||
| function readDefaultsFromEnv(): Record<string, Record<string, unknown>> { | ||
| const raw = process.env.SFNEXT_DATA_STORE_DEFAULTS; | ||
| if (!raw) { | ||
| return {}; | ||
| } | ||
|
|
||
| try { | ||
| const parsed: unknown = JSON.parse(raw); | ||
| if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { | ||
| return parsed as Record<string, Record<string, unknown>>; | ||
| } | ||
| } catch (error) { | ||
| console.warn('Failed to parse SFNEXT_DATA_STORE_DEFAULTS JSON.', error); | ||
| } | ||
|
|
||
| return {}; | ||
| } | ||
|
|
||
| function readWarnOnMissingFromEnv(): boolean { | ||
| const raw = process.env.SFNEXT_DATA_STORE_WARN_ON_MISSING; | ||
| if (!raw) { | ||
| return true; | ||
| } | ||
|
|
||
| return raw.toLowerCase() !== 'false'; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| /* | ||
| * Copyright (c) 2025, Salesforce, Inc. | ||
| * SPDX-License-Identifier: Apache-2 | ||
| * For full license text, see the license.txt file in the repo root or http://www.apache.org/licenses/LICENSE-2.0 | ||
| */ | ||
|
|
||
| export class DataStoreNotFoundError extends Error { | ||
| constructor(message: string) { | ||
| super(message); | ||
| this.name = 'DataStoreNotFoundError'; | ||
| Object.setPrototypeOf(this, DataStoreNotFoundError.prototype); | ||
| } | ||
| } | ||
|
|
||
| export class DataStoreServiceError extends Error { | ||
| constructor(message: string) { | ||
| super(message); | ||
| this.name = 'DataStoreServiceError'; | ||
| Object.setPrototypeOf(this, DataStoreServiceError.prototype); | ||
| } | ||
| } | ||
|
|
||
| export class DataStoreUnavailableError extends Error { | ||
| constructor(message: string) { | ||
| super(message); | ||
| this.name = 'DataStoreUnavailableError'; | ||
| Object.setPrototypeOf(this, DataStoreUnavailableError.prototype); | ||
| } | ||
| } |
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.
don't forget to remove this file before merging?