Skip to content

Commit

Permalink
Improve Pointer type to be more precise
Browse files Browse the repository at this point in the history
The Pointer type has been updated to reflect the result of Pointer.from,
returning never if the type does not exist, or undefined if the parent
exists.

Change-type: minor
  • Loading branch information
pipex committed Jul 18, 2024
1 parent 5e06537 commit 1ae12d7
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 23 deletions.
7 changes: 3 additions & 4 deletions lib/agent/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import type { Task } from '../task';
import { Runtime } from './runtime';
import type { AgentOpts, Result } from './types';
import { NotStarted } from './types';
import type { Path } from '../path';
import { patch } from './patch';
import type { Operation } from '../operation';

Expand Down Expand Up @@ -179,13 +178,13 @@ function from<TState>(
| {
initial: TState;
planner?: Planner<TState>;
sensors?: Array<Sensor<TState, Path>>;
sensors?: Array<Sensor<TState>>;
opts?: DeepPartial<AgentOpts<TState>>;
}
| {
initial: TState;
tasks?: Array<Task<TState, any, any>>;
sensors?: Array<Sensor<TState, Path>>;
sensors?: Array<Sensor<TState>>;
opts?: DeepPartial<AgentOpts<TState>>;
},
): Agent<TState>;
Expand All @@ -201,7 +200,7 @@ function from<TState>({
initial: TState;
tasks?: Array<Task<TState, any, any>>;
planner?: Planner<TState>;
sensors?: Array<Sensor<TState, Path>>;
sensors?: Array<Sensor<TState>>;
opts?: DeepPartial<AgentOpts<TState>>;
}): Agent<TState> {
const opts: AgentOpts<TState> = {
Expand Down
2 changes: 1 addition & 1 deletion lib/agent/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export class Runtime<TState> {
private stateRef: Ref<TState>;

constructor(
private readonly observer: Observer<Operation<TState, Path>>,
private readonly observer: Observer<Operation<TState>>,
state: TState,
private readonly target: Target<TState> | StrictTarget<TState>,
private readonly planner: Planner<TState>,
Expand Down
2 changes: 1 addition & 1 deletion lib/distance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ function* getOperations<S>(s: S, t: Target<S>): Iterable<TreeOperation<S>> {

const path = Path.from(ref);
const sValue = Pointer.from(s, path);
const tValue = Pointer.from(patched, path);
const tValue = tgt !== UNDEFINED ? Pointer.from(patched, path) : undefined;

// If the target is DELETED, and the source value still
// exists we need to add a delete operation
Expand Down
16 changes: 16 additions & 0 deletions lib/path.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,20 @@ describe('Path', () => {
expect(Path.from('/a/b_c-123#/1')).to.equal('/a/b_c-123#/1');
});
});

describe('source', () => {
it('returns the parent path', () => {
expect(Path.source(Path.from('/a/b/c'))).to.equal('/a/b');
expect(Path.source(Path.from('/a'))).to.equal('/');
expect(Path.source(Path.from('/'))).to.equal('/');
});
});

describe('basename', () => {
it('returns the basename of the path', () => {
expect(Path.basename(Path.from('/a/b/c'))).to.equal('c');
expect(Path.basename(Path.from('/a'))).to.equal('a');
expect(Path.basename(Path.from('/'))).to.equal('');
});
});
});
10 changes: 5 additions & 5 deletions lib/planner/findPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,11 +395,11 @@ export function findPlan<TState = any>({
// we get the target value for the context from the pointer
// if the operation is delete, the pointer will be undefined
// which is the right value for that operation
const ctx = Lens.context<TState, string>(
task.lens,
path,
Pointer.from<TState, string>(distance.target, path),
);
const tgt =
operation.op === 'delete'
? undefined
: Pointer.from<TState, string>(distance.target, path);
const ctx = Lens.context<TState, string>(task.lens, path, tgt);

const taskPlan = tryInstruction(task(ctx), {
depth,
Expand Down
2 changes: 2 additions & 0 deletions lib/pointer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@ describe('Pointer', () => {
).to.deep.equal({ a: 1, b: { c: 2, d: { e: 'hello' } } });

expect(
// eslint-disable-next-line @typescript-eslint/no-confusing-void-expression
Pointer.from({ a: 1, b: { c: 2, d: { e: 'hello' } } }, Path.from('/x')),
).to.be.undefined;
expect(
// eslint-disable-next-line @typescript-eslint/no-confusing-void-expression
Pointer.from(
{ a: 1, b: { c: 2, d: { e: 'hello' } } },
Path.from('/b/d/x'),
Expand Down
36 changes: 29 additions & 7 deletions lib/pointer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,18 @@ import { isArrayIndex } from './is-array-index';

export type Pointer<O, P extends PathType> = PointerWithSlash<O, P>;

type IsLiteral<T> = T extends string
? string extends T
? false
: true
: false;

type PointerWithSlash<O, P extends PathType> = P extends `/${infer R}`
? PointerWithoutSlash<O, R>
: unknown;
: IsLiteral<P> extends true
? never
: // If the string is not a literal we cannot know what the pointer type is
unknown;
type PointerWithoutSlash<
O,
P extends PathType,
Expand All @@ -26,7 +35,9 @@ type PointerWithSinglePath<O, H extends string> = O extends any[]
? O[number]
: H extends keyof O
? O[H]
: never;
: H extends ''
? O
: undefined;

export class InvalidPointer extends Error {
constructor(path: PathType, obj: unknown) {
Expand All @@ -42,6 +53,9 @@ function from<O = any, P extends PathType = Root>(
): Pointer<O, P> {
const parts = Path.split(path);

// Save the last element of the path so we can delete it
const last = parts.pop();

let o = obj as any;
for (const p of parts) {
if (!Array.isArray(o) && typeof o !== 'object') {
Expand All @@ -52,16 +66,24 @@ function from<O = any, P extends PathType = Root>(
throw new InvalidPointer(path, obj);
}

// TODO: should we make this more restrictive? Perhaps throw unless the parent
// exists like in view?
// Pointer is permissive, if the object does not exist in the type,
// it doesn't mean it cannot exist so we return undefined
if (!(p in o)) {
return undefined as Pointer<O, P>;
throw new InvalidPointer(path, obj);
}
o = o[p];
}

if (last != null) {
if (!Array.isArray(o) && typeof o !== 'object') {
throw new InvalidPointer(path, obj);
}

if (Array.isArray(o) && !isArrayIndex(last)) {
throw new InvalidPointer(path, obj);
}

return o[last];
}

return o;
}

Expand Down
6 changes: 3 additions & 3 deletions lib/sensor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ import { Observable } from './observable';
* A Sensor function for type T is a function that returns a generator
* that yields values of type T
*/
type SensorFn<T, P extends PathType = '/'> = (
type SensorFn<T, P extends PathType = PathType> = (
args: LensArgs<T, P>,
) =>
| AsyncGenerator<Lens<T, P>, void, void>
| Generator<Lens<T, P>, void, void>
| Subscribable<Lens<T, P>>;

type SensorOutput<T, P extends PathType = '/'> = Subscribable<
type SensorOutput<T, P extends PathType = PathType> = Subscribable<
UpdateOperation<T, P>
>;

Expand All @@ -27,7 +27,7 @@ type SensorOutput<T, P extends PathType = '/'> = Subscribable<
* returns a subscribable that allows to observe changes
* to the state returned by the sensor operation.
*/
export type Sensor<T, P extends PathType = '/'> =
export type Sensor<T, P extends PathType = PathType> =
unknown extends Lens<T, P>
? // Default to the version with path if the lens cannot be resolved
{ (path: PathType): SensorOutput<T, P>; lens: Path<P> }
Expand Down
12 changes: 10 additions & 2 deletions tests/orchestrator/tasks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ describe('orchestrator/tasks', () => {
apps: {
a0: {
name: 'my-app',
releases: {},
releases: {
r0: {
services: {},
},
},
},
},
images: {},
Expand All @@ -76,7 +80,11 @@ describe('orchestrator/tasks', () => {
apps: {
a0: {
name: 'my-app',
releases: {},
releases: {
r0: {
services: {},
},
},
},
},
images: {},
Expand Down

0 comments on commit 1ae12d7

Please sign in to comment.