Skip to content

Commit f8e35f5

Browse files
committed
update
1 parent 81d6f86 commit f8e35f5

3 files changed

Lines changed: 120 additions & 2 deletions

File tree

.agent/rules/porting.mdc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ TypeScript:
5252

5353
| Go | TypeScript |
5454
| ---------------------------- | ----------------------------------- |
55+
| `databricks/api/` | `packages/databricks/src/api/` |
5556
| `databricks/apierr/` | `packages/databricks/src/apierror/` |
5657
| `databricks/apierr/codes/` | `packages/databricks/src/apierror/codes/` |
5758

@@ -138,6 +139,22 @@ tc.want.httpErr" or "equivalent of Go's errors.As."
138139
// Input HTTP fields are always preserved.
139140
```
140141

142+
## Value Types vs Reference Types
143+
144+
Go structs are value types — passing a struct to a function copies it. TypeScript
145+
objects are reference types — passing an object shares it. When Go code relies on
146+
value-copy semantics (e.g. a retrier getting its own copy of a backoff policy),
147+
the TypeScript port must account for this. Common approaches:
148+
149+
1. **Accept options, create internally** — the function takes a plain options
150+
object and creates its own instance. This is the most ergonomic.
151+
2. **Clone internally** — the function accepts an instance but clones it.
152+
3. **Document the constraint** — accept an instance, document that each caller
153+
must provide a fresh one. Simple but error-prone.
154+
155+
Do not silently share mutable state between callers. Explain the chosen approach
156+
to the user as a deviation.
157+
141158
## Deviations from Go
142159

143160
When the TypeScript implementation must differ from Go (e.g. different function

.agent/rules/testing.mdc

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,30 @@ const testCases: {desc: string; want?: {code: Code; message: string}}[] = [
129129
];
130130
```
131131

132+
## Mocking Module-Level Functions
133+
134+
To mock a module-level function in ESM, the function must be a method on an
135+
exported object — not a standalone exported function. Standalone functions are
136+
captured as local closure references and `vi.spyOn` cannot intercept them.
137+
138+
```typescript
139+
// In the source file:
140+
export const rand = {
141+
int(n: number): number {
142+
return Math.floor(Math.random() * n);
143+
},
144+
};
145+
146+
// In the test file:
147+
import {rand} from '../../src/api/retrier';
148+
149+
vi.spyOn(rand, 'int').mockImplementation(n => n - 1);
150+
// ... run test ...
151+
vi.restoreAllMocks();
152+
```
153+
154+
Always call `vi.restoreAllMocks()` after mocking to avoid test pollution.
155+
132156
## Independence
133157

134158
Each test must be independent. Do not rely on execution order. Clean up side

.agent/rules/typescript.mdc

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -443,11 +443,88 @@ Avoid private static methods; prefer module-level functions instead. Never use
443443

444444
Always include parentheses in constructor calls: `new Foo()`, not `new Foo`.
445445

446-
### 7.7 API Surface Minimization
446+
### 7.7 Internal Methods and `@private` JSDoc
447+
448+
When a method or constructor must be accessible to internal code and tests but
449+
should not be used by consumers, mark it with `@private` JSDoc. Do not export
450+
the options interface from `index.ts`.
451+
452+
```typescript
453+
/**
454+
* Do not use this constructor directly. Use {@link MyClass.create} instead.
455+
* This constructor is only meant for internal and testing use.
456+
*
457+
* @private
458+
*/
459+
constructor(options: MyClassOptions) { ... }
460+
```
461+
462+
**When users need to instantiate a class directly** (no factory function), the
463+
constructor must be public and the options interface **must** be exported from
464+
`index.ts`. A public constructor with an unexported options type gives consumers
465+
no autocomplete and no documentation — that is a broken experience. Only use
466+
`@private` JSDoc when there is a dedicated factory function (e.g.
467+
`fromHttpError`).
468+
469+
### 7.8 API Surface Minimization
447470

448471
Default to the smallest public API surface. Only export from `index.ts` what
449472
consumers need. Internal types (options interfaces, schemas, helper functions)
450-
must not be re-exported.
473+
must not be re-exported. Use `@private` JSDoc for methods that are technically
474+
accessible but not part of the public contract.
475+
476+
### 7.9 Consumer Experience
477+
478+
When creating any new public export, always consider:
479+
480+
- Can the consumer see the types? (Are options interfaces exported?)
481+
- Do they get autocomplete and JSDoc in their IDE?
482+
- Is it clear how to use the API without reading the source?
483+
484+
Provide consumer usage examples to the user when proposing new public APIs.
485+
486+
### 7.10 Testing Hooks
487+
488+
Testing hooks (e.g. a custom random number generator for deterministic tests)
489+
must never appear in constructor options or public interfaces. Instead, use a
490+
module-level object that tests can mock with `vi.spyOn`.
491+
492+
Standalone module-level functions cannot be mocked with `vi.spyOn` in ESM.
493+
In ESM, when a module defines and calls its own function, the internal call is
494+
bound to the function's local variable at module evaluation time — not to the
495+
module's export binding. `vi.spyOn(module, 'fn')` replaces the export binding,
496+
but the class's method still holds a closure over the original function. The
497+
spy intercepts what *importers* see, but not what the module's *own code* calls.
498+
499+
Wrapping the function in an object (`rand.int()`) fixes this because the call
500+
goes through a property lookup on the object at runtime. `vi.spyOn(rand, 'int')`
501+
replaces the property on the object itself, so any code that calls `rand.int()`
502+
— including internal code in the same module — sees the mock.
503+
504+
```typescript
505+
// Good — testable via vi.spyOn(rand, 'int').
506+
export const rand = {
507+
int(n: number): number {
508+
return Math.floor(Math.random() * n);
509+
},
510+
};
511+
512+
class BackoffPolicy {
513+
delay(): number {
514+
return rand.int(this.current + 1);
515+
}
516+
}
517+
518+
// Bad — randomInt in constructor options pollutes the public API.
519+
interface BackoffPolicyOptions {
520+
randomInt?: (n: number) => number; // Do not do this.
521+
}
522+
523+
// Bad — standalone function cannot be mocked in ESM.
524+
export function randomInt(n: number): number { ... }
525+
```
526+
527+
Do not export testing objects from `index.ts` — they are internal.
451528

452529
---
453530

0 commit comments

Comments
 (0)