I recently constructed a Builder
class, and I realized that some of the fields should be mandatory, before tying up the builder chain with the final execute()
.
I figured this check could be done statically at compile-time, and as such I came up with this solution (simplified for the sake of the example):
interface abc {
a: string; // required
b: string; // required
c: string; // optional
}
class Builder<T = void> {
protected state: T = {} as any; // necessary ugly cast
public a(aa: string): Builder<T & Pick<abc, 'a'>> {
Object.defineProperty(this.state, 'a', {
value: aa,
writable: true,
});
return this as unknown as Builder<T & Pick<abc, 'a'>>;
}
public b(bb: string): Builder<T & Pick<abc, 'b'>> {
Object.defineProperty(this.state, 'b', {
value: bb,
writable: true,
});
return this as unknown as Builder<T & Pick<abc, 'b'>>;
}
public c(cc: string): Builder<T & Pick<abc, 'c'>> {
Object.defineProperty(this.state, 'c', {
value: cc,
writable: true,
});
return this as unknown as Builder<T & Pick<abc, 'c'>>;
}
public execute(this: Builder<Pick<abc, 'a' | 'b'>>) {
return this.state;
}
}
Now, when I do this:
new Builder().a('foo').c('bar').execute();
I get the error I desire, which is: The 'this' context of type ... is not assignable to method's 'this' of type ...
, which is exactly what I wanted.
However, when I compile this typescript code and import the build files in another project, I don't get this error anymore.
This is the compiled type definition:
interface abc {
a: string;
b: string;
c: string;
}
declare class Builder<T> {
protected state: T;
a(aa: string): Builder<T & Pick<abc, 'a'>>;
b(bb: string): Builder<T & Pick<abc, 'b'>>;
c(cc: string): Builder<T & Pick<abc, 'c'>>;
execute(this: Builder<Pick<abc, 'a' | 'b'>>): Pick<abc, "a" | "b">;
}
And here is my tsconfig.json (I've excluded the project paths to stay relevant to the topic):
{
"compilerOptions": {
"target": "ESNext",
"module": "ESNext",
"strict": true,
"importHelpers": true,
"declaration": true,
"declarationDir": "dist/esm/types/",
"moduleResolution": "node",
"experimentalDecorators": true,
"skipLibCheck": true,
"esModuleInterop": true,
"allowSyntheticDefaultImports": true,
"sourceMap": true,
"baseUrl": ".",
"noImplicitAny": true,
"lib": [
"ESNext",
"webworker",
"DOM"
]
}
}
To add insult to injury, IntelliSense shows me these types when hovering over the variables (although I know this is not necessarily tied to the compiler):
new Builder().execute(); // should be an error
// with the type...
Builder<void>.send(this: Builder<Pick<abc, "a" | "b">>): ...
Is this intended behaviour? Do I need to change tsconfig.json? Am I doing something wrong? If I don't compile the class, then it works perfectly.
CodePudding user response:
You should be aware that this line is UNSAFE: protected state: T = {} as any;
.
Please consider this approach:
class Builder {
state = {}
public a<Param extends string>(aa: Param) {
Object.assign(this.state, { a: aa })
return this as this & { state: { a: Param } }
}
public b<Param extends string>(bb: Param) {
Object.assign(this.state, { b: bb })
return this as this & { state: { b: Param } }
}
public c<Param extends string>(cc: Param) {
Object.assign(this.state, { c: cc })
return this as this & { state: { c: Param } }
}
public execute() {
return this.state;
}
}
new Builder().a('foo').state.a // foo
new Builder().a('foo').b('bar').state.b // bar
CodePudding user response:
With help from captain-yossarian, I realized what went wrong and why I encountered the problem.
The problem with my approach
After two years of playing around with types, I had an incorrect separation of type and entity in my head. In reality, they are much more neatly intertwined within TypeScript. While using generics is a great approach for conditional-recursive-mapped types, this use case was much simpler, as captain-yossarian proved.
I didn't need to box a type within a generic just to progressively modify it within each subsequent call. this
is in itself a type, and it proved most helpful here.
A class is, after all, just another type. So I didn't need some external interface to build as I built the entity. I could've just used the entity itself to represent the 'progressive' type.
(If you have such an external interface, though, Pick<Type, Keys> works great with this use case)
The solution, after applying what was suggested
The solution is really similar to what captain-yossarian suggested. A few lines were added to finish the solution to my question.
I used another interface because it's the only implementation I actually tested. It is also a bit easier to comprehend at a first glance.
interface ABC {
a: string;
b: string;
c: string;
}
class Builder {
state = {};
public a(aa: string) {
Object.assign(this.state, { a: aa });
return this as this & { state: Pick<ABC, 'a'> };
}
public b(bb: Param) {
Object.assign(this.state, { b: bb });
return this as this & { state: Pick<ABC, 'b'> };
}
public c(cc: Param) {
Object.assign(this.state, { c: cc });
return this as this & { state: Pick<ABC, 'c'> };
}
public execute(this: this & { state: Pick<ABC, 'a' | 'b'> }) {
return this.state;
}
}
The good part is that this also works after it has been compiled. A general 'win' situation happened, but...
The downside
You can't make the state private
, or the type intersection will produce never
(because both this
and your interface provide a value for state
, and one is private). The generic approach, although more clumsy, did work well with private fields. In this sense, we could just use a state for marking the progress, but then what would be the point of type checking, if we're not typing the states directly?
What happened
That I can't really tell. I'm sure that the compiler shouldn't produce results that work differently from when the code is not compiled. I'm not sure what was lost on the way, but I'm curious to find out more about this. Like with many other things, maybe this will be fixed.
Conclusion
Types should/can mainly be used to model the data. This is their primary use, and it is a powerful tool when used correctly. I think using advanced types for interesting operations is useful, but sometimes (as it was in this case) doing the entire mechanism that way was overkill.