Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: Factory types #1812

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: Factory types #1812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 98 additions & 17 deletions src/Factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,75 @@
import { Node } from './Node';
import { GetSet } from './types';
import { Util } from './Util';
import { getComponentValidator } from './Validators';

var GET = 'get',
SET = 'set';
const GET = 'get';
const SET = 'set';

/**
* Enforces that a type is a string.
*/
type EnforceString<T> = T extends string ? T : never;

/**
* Represents a class.
*/
type Constructor = abstract new (...args: any) => any;

/**
* An attribute of an instance of the provided class. Attributes names be strings.
*/
type Attr<T extends Constructor> = EnforceString<keyof InstanceType<T>>;

/**
* A function that is called after a setter is called.
*/
type AfterFunc<T extends Constructor> = (this: InstanceType<T>) => void;

/**
* Extracts the type of a GetSet.
*/
type ExtractGetSet<T> = T extends GetSet<infer U, any> ? U : never;

/**
* Extracts the type of a GetSet class attribute.
*/
type Value<T extends Constructor, U extends Attr<T>> = ExtractGetSet<
InstanceType<T>[U]
>;

/**
* A function that validates a value.
*/
type ValidatorFunc<T> = (val: ExtractGetSet<T>, attr: string) => T;

/**
* Extracts the "components" (keys) of a GetSet value. The value must be an object.
*/
type ExtractComponents<T extends Constructor, U extends Attr<T>> = Value<
T,
U
> extends Record<string, any>
? EnforceString<keyof Value<T, U>>[]
: never;

export const Factory = {
addGetterSetter(constructor, attr, def?, validator?, after?) {
addGetterSetter<T extends Constructor, U extends Attr<T>>(
constructor: T,
attr: U,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can make this a bit safer with a type to constrain attr to be a GetSet.

type EnforceGetSet<
  T extends Constructor,
  U extends Attr<T>
> = InstanceType<T>[U] extends GetSet<any, any> ? U : never;

Then, type attr as attr: EnforceGetSet<T, U>.

However, this causes a problem in Canvas.ts at https://github.com/psychedelicious/konva/blob/9c41a6eed1bba46e80ce282903ac86bb11493e84/src/Canvas.ts#L171

def?: Value<T, U>,
validator?: ValidatorFunc<Value<T, U>>,
after?: AfterFunc<T>
): void {
Factory.addGetter(constructor, attr, def);
Factory.addSetter(constructor, attr, validator, after);
Factory.addOverloadedGetterSetter(constructor, attr);
},
addGetter(constructor, attr, def?) {
addGetter<T extends Constructor, U extends Attr<T>>(
constructor: T,
attr: U,
def?: Value<T, U>
) {
var method = GET + Util._capitalize(attr);

constructor.prototype[method] =
Expand All @@ -22,14 +80,25 @@ export const Factory = {
};
},

addSetter(constructor, attr, validator?, after?) {
addSetter<T extends Constructor, U extends Attr<T>>(
constructor: T,
attr: U,
validator?: ValidatorFunc<Value<T, U>>,
after?: AfterFunc<T>
) {
var method = SET + Util._capitalize(attr);

if (!constructor.prototype[method]) {
Factory.overWriteSetter(constructor, attr, validator, after);
}
},
overWriteSetter(constructor, attr, validator?, after?) {

overWriteSetter<T extends Constructor, U extends Attr<T>>(
constructor: T,
attr: U,
validator?: ValidatorFunc<Value<T, U>>,
after?: AfterFunc<T>
) {
var method = SET + Util._capitalize(attr);
constructor.prototype[method] = function (val) {
if (validator && val !== undefined && val !== null) {
Expand All @@ -45,12 +114,13 @@ export const Factory = {
return this;
};
},
addComponentsGetterSetter(
constructor,
attr: string,
components: Array<string>,
validator?: Function,
after?: Function

addComponentsGetterSetter<T extends Constructor, U extends Attr<T>>(
constructor: T,
attr: U,
components: ExtractComponents<T, U>,
validator?: ValidatorFunc<Value<T, U>>,
after?: AfterFunc<T>
) {
var len = components.length,
capitalize = Util._capitalize,
Expand Down Expand Up @@ -79,7 +149,7 @@ export const Factory = {
key;

if (validator) {
val = validator.call(this, val);
val = validator.call(this, val, attr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this change do?

}

if (basicValidator) {
Expand Down Expand Up @@ -109,7 +179,10 @@ export const Factory = {

Factory.addOverloadedGetterSetter(constructor, attr);
},
addOverloadedGetterSetter(constructor, attr) {
addOverloadedGetterSetter<T extends Constructor, U extends Attr<T>>(
constructor: T,
attr: U
) {
var capitalizedAttr = Util._capitalize(attr),
setter = SET + capitalizedAttr,
getter = GET + capitalizedAttr;
Expand All @@ -124,7 +197,12 @@ export const Factory = {
return this[getter]();
};
},
addDeprecatedGetterSetter(constructor, attr, def, validator) {
addDeprecatedGetterSetter<T extends Constructor, U extends Attr<T>>(
constructor: T,
attr: U,
def: Value<T, U>,
validator: ValidatorFunc<Value<T, U>>
) {
Util.error('Adding deprecated ' + attr);

var method = GET + Util._capitalize(attr);
Expand All @@ -142,7 +220,10 @@ export const Factory = {
});
Factory.addOverloadedGetterSetter(constructor, attr);
},
backCompat(constructor, methods) {
backCompat<T extends Constructor>(
constructor: T,
methods: Record<string, string>
) {
Util.each(methods, function (oldMethodName, newMethodName) {
var method = constructor.prototype[newMethodName];
var oldGetter = GET + Util._capitalize(oldMethodName);
Expand All @@ -164,7 +245,7 @@ export const Factory = {
constructor.prototype[oldSetter] = deprecated;
});
},
afterSetFilter(this: Node) {
afterSetFilter(this: Node): void {
this._filterUpToDate = false;
},
};
6 changes: 3 additions & 3 deletions src/Node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2660,7 +2660,7 @@ export abstract class Node<Config extends NodeConfig = NodeConfig> {
rotation: GetSet<number, this>;
zIndex: GetSet<number, this>;

scale: GetSet<Vector2d | undefined, this>;
scale: GetSet<Vector2d, this>;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either this GetSet type is incorrect for the NodeConfig.scale type is incorrect. I assume it's this one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it is not correct, I don't think scale can return undefined.

scaleX: GetSet<number, this>;
scaleY: GetSet<number, this>;
skew: GetSet<Vector2d, this>;
Expand Down Expand Up @@ -3102,7 +3102,7 @@ addGetterSetter(Node, 'offsetY', 0, getNumberValidator());
* node.offsetY(3);
*/

addGetterSetter(Node, 'dragDistance', null, getNumberValidator());
addGetterSetter(Node, 'dragDistance', undefined, getNumberValidator());
Copy link
Author

@psychedelicious psychedelicious Aug 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a subtle API change.

Perhaps we should widen the type of dragDistance in NodeConfig to dragDistance?: number | null;?

There are a number of other places where the initial value is set to null apparently erroneously.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The drag distance must be always number. So undefined should fallback to default value. I am not really sure what it was null here.


/**
* get/set drag distance
Expand Down Expand Up @@ -3193,7 +3193,7 @@ addGetterSetter(Node, 'listening', true, getBooleanValidator());

addGetterSetter(Node, 'preventDefault', true, getBooleanValidator());

addGetterSetter(Node, 'filters', null, function (this: Node, val) {
addGetterSetter(Node, 'filters', undefined, function (this: Node, val) {
this._filterUpToDate = false;
return val;
});
Expand Down
5 changes: 5 additions & 0 deletions src/Shape.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,11 @@ export class Shape<
strokeLinearGradientStartPoint: GetSet<Vector2d, this>;
strokeLinearGradientEndPoint: GetSet<Vector2d, this>;
strokeLinearGradientColorStops: GetSet<Array<number | string>, this>;
strokeLinearGradientStartPointX: GetSet<number, this>;
strokeLinearGradientStartPointY: GetSet<number, this>;
strokeLinearGradientEndPointX: GetSet<number, this>;
strokeLinearGradientEndPointY: GetSet<number, this>;
fillRule: GetSet<CanvasFillRule, this>;
Comment on lines +846 to +850
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These annotations were missing. I don't think this changes anything because the Factory adds the GetSets anyways.

}

Shape.prototype._fillFunc = _fillFunc;
Expand Down
38 changes: 19 additions & 19 deletions src/Validators.ts
View file
Open in desktop
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I have understood the intention with these validators - they should create warnings but otherwise let the value through, even if it is invalid.

Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ export function alphaComponent(val: number) {
return val;
}

export function getNumberValidator() {
export function getNumberValidator<T>() {
if (Konva.isUnminified) {
return function <T>(val: T, attr: string): T | void {
return function (val: T, attr: string): T {
if (!Util._isNumber(val)) {
Util.warn(
_formatValue(val) +
Expand All @@ -49,9 +49,9 @@ export function getNumberValidator() {
}
}

export function getNumberOrArrayOfNumbersValidator(noOfElements: number) {
export function getNumberOrArrayOfNumbersValidator<T>(noOfElements: number) {
if (Konva.isUnminified) {
return function <T>(val: T, attr: string): T | void {
return function (val: T, attr: string): T {
let isNumber = Util._isNumber(val);
let isValidArray = Util._isArray(val) && val.length == noOfElements;
if (!isNumber && !isValidArray) {
Expand All @@ -69,9 +69,9 @@ export function getNumberOrArrayOfNumbersValidator(noOfElements: number) {
}
}

export function getNumberOrAutoValidator() {
export function getNumberOrAutoValidator<T>() {
if (Konva.isUnminified) {
return function <T extends string>(val: T, attr: string): T | void {
return function (val: T, attr: string): T {
var isNumber = Util._isNumber(val);
var isAuto = val === 'auto';

Expand All @@ -88,9 +88,9 @@ export function getNumberOrAutoValidator() {
}
}

export function getStringValidator() {
export function getStringValidator<T>() {
if (Konva.isUnminified) {
return function (val: any, attr: string) {
return function (val: T, attr: string): T {
if (!Util._isString(val)) {
Util.warn(
_formatValue(val) +
Expand All @@ -104,13 +104,13 @@ export function getStringValidator() {
}
}

export function getStringOrGradientValidator() {
export function getStringOrGradientValidator<T>() {
if (Konva.isUnminified) {
return function (val: any, attr: string) {
return function (val: T, attr: string): T {
const isString = Util._isString(val);
const isGradient =
Object.prototype.toString.call(val) === '[object CanvasGradient]' ||
(val && val.addColorStop);
(val && val['addColorStop']);
if (!(isString || isGradient)) {
Util.warn(
_formatValue(val) +
Expand All @@ -124,9 +124,9 @@ export function getStringOrGradientValidator() {
}
}

export function getFunctionValidator() {
export function getFunctionValidator<T>() {
if (Konva.isUnminified) {
return function (val: any, attr: string) {
return function (val: T, attr: string): T {
if (!Util._isFunction(val)) {
Util.warn(
_formatValue(val) +
Expand All @@ -139,9 +139,9 @@ export function getFunctionValidator() {
};
}
}
export function getNumberArrayValidator() {
export function getNumberArrayValidator<T>() {
if (Konva.isUnminified) {
return function (val: any, attr: string) {
return function (val: T, attr: string): T {
// Retrieve TypedArray constructor as found in MDN (if TypedArray is available)
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray#description
const TypedArray = Int8Array ? Object.getPrototypeOf(Int8Array) : null;
Expand Down Expand Up @@ -172,9 +172,9 @@ export function getNumberArrayValidator() {
};
}
}
export function getBooleanValidator() {
export function getBooleanValidator<T>() {
if (Konva.isUnminified) {
return function (val: any, attr: string) {
return function (val: T, attr: string): T {
var isBool = val === true || val === false;
if (!isBool) {
Util.warn(
Expand All @@ -188,9 +188,9 @@ export function getBooleanValidator() {
};
}
}
export function getComponentValidator(components: any) {
export function getComponentValidator<T>(components: string[]) {
if (Konva.isUnminified) {
return function (val: any, attr: string) {
return function (val: T, attr: string): T {
// ignore validation on undefined value, because it will reset to defalt
if (val === undefined || val === null) {
return val;
Expand Down
4 changes: 2 additions & 2 deletions src/filters/Emboss.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ Factory.addGetterSetter(
Node,
'embossDirection',
'top-left',
null,
undefined,
Factory.afterSetFilter
);
/**
Expand All @@ -190,7 +190,7 @@ Factory.addGetterSetter(
Node,
'embossBlend',
false,
null,
undefined,
Factory.afterSetFilter
);
/**
Expand Down
5 changes: 4 additions & 1 deletion src/shapes/Label.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,10 @@ export class Tag extends Shape<TagConfig> {
};
}

pointerDirection: GetSet<'left' | 'top' | 'right' | 'bottom', this>;
pointerDirection: GetSet<
'left' | 'top' | 'right' | 'bottom' | typeof NONE,
this
>;
Comment on lines +320 to +323
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The factory sets the initial value to NONE.

pointerWidth: GetSet<number, this>;
pointerHeight: GetSet<number, this>;
cornerRadius: GetSet<number, this>;
Expand Down
4 changes: 2 additions & 2 deletions src/shapes/TextPath.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ Factory.addGetterSetter(TextPath, 'text', EMPTY_STRING);
* // underline text
* shape.textDecoration('underline');
*/
Factory.addGetterSetter(TextPath, 'textDecoration', null);
Factory.addGetterSetter(TextPath, 'textDecoration', undefined);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably should be an empty string as in Text class.


/**
* get/set kerning function.
Expand All @@ -568,4 +568,4 @@ Factory.addGetterSetter(TextPath, 'textDecoration', null);
* return 1;
* });
*/
Factory.addGetterSetter(TextPath, 'kerningFunc', null);
Factory.addGetterSetter(TextPath, 'kerningFunc', undefined);
2 changes: 0 additions & 2 deletions src/shapes/Transformer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1745,8 +1745,6 @@ Factory.addGetterSetter(Transformer, 'ignoreStroke', false);
*/
Factory.addGetterSetter(Transformer, 'padding', 0, getNumberValidator());

Factory.addGetterSetter(Transformer, 'node');

Comment on lines -1748 to -1749
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was added accidentally - Transformer never references this.node as far as I can see...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an old (deprecated) API. Should be safe to remove.

/**
* get/set attached nodes of the Transformer. Transformer will adapt to their size and listen to their events
* @method
Expand Down
Loading