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

Refactor property value and default function resolution #172

Merged
merged 8 commits into from
Jul 2, 2024
Merged

Refactor property value and default function resolution #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
98b6e8d
feat(core): prefer `prop.value` when `prop.default` is not a function
SgLy Jun 28, 2024
148d144
feat(core): ensure prop value not to be undefined when `prop.default`…
SgLy Jun 28, 2024
4404fa0
feat(core): ensure prop value not to be undefined when `prop.default`…
SgLy Jun 28, 2024
66d80cc
feat(core): move typeof `prop.default` call to prepare period
SgLy Jun 28, 2024
d4cfd53
Merge branch 'master' into feat-better-prop-default-protection
SgLy Jun 28, 2024
1e55402
refactor(core): process `prop.default` and `prop.value` during behavi…
SgLy Jun 28, 2024
bda1dfa
fix(core): fallbackValue should be copied for default / initialValue
SgLy Jun 28, 2024
5181d58
test(core): add more test cases for property value and default resolu…
SgLy Jun 28, 2024
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
42 changes: 29 additions & 13 deletions glass-easel/src/behavior.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { parseMultiPaths, type MultiPaths } from './data_path'
import {
DataGroupObserverTree,
NormalizedPropertyType,
getPropertyFallbackValue,
normalizePropertyType,
normalizePropertyTypeShortHand,
shallowMerge,
Expand Down Expand Up @@ -1187,13 +1188,15 @@ export class Behavior<
// init properties
const properties = builder._$properties
if (properties !== undefined) {
const initValueFuncs: { name: string; func: (this: null) => any }[] = []
const initValueFuncs: { name: string; func: () => any }[] = []
for (let i = 0; i < properties.length; i += 1) {
const { name, def } = properties[i]!
const shortHandDef = normalizePropertyTypeShortHand(def)
let d: PropertyDefinition
let initialValueFn: () => DataValue
if (shortHandDef !== null) {
d = shortHandDef
initialValueFn = shortHandDef.defaultFn
} else {
const propDef = def as PropertyOption<any, unknown>
let type = normalizePropertyType(propDef.type)
Expand All @@ -1209,19 +1212,33 @@ export class Behavior<
if (type === NormalizedPropertyType.Invalid) {
dispatchError(new Error(`the type of property "${name}" is illegal`), `[prepare]`, is)
}
let value: DataValue = propDef.value
if (propDef.value === undefined) {
if (type === NormalizedPropertyType.String) value = ''
else if (type === NormalizedPropertyType.Number) value = 0
else if (type === NormalizedPropertyType.Boolean) value = false
else if (type === NormalizedPropertyType.Array) value = []
else value = null
} else if (propDef.default !== undefined) {
const fallbackValue = getPropertyFallbackValue(type)
if (typeof propDef.default === 'function' && propDef.value !== undefined) {
triggerWarning(
`the initial value of property "${name}" is not used when its default is provided.`,
is,
)
}
const defaultFn =
typeof propDef.default === 'function'
? () => {
const value = safeCallback(
`Property "${name}" Default`,
propDef.default!,
null,
[],
is,
)
return value !== undefined ? value : simpleDeepCopy(fallbackValue)
}
: () => simpleDeepCopy(fallbackValue)
if (typeof propDef.default === 'function') {
initialValueFn = defaultFn
} else if (propDef.value !== undefined) {
initialValueFn = () => simpleDeepCopy(propDef.value)
} else {
initialValueFn = () => simpleDeepCopy(fallbackValue)
}
let observer: ((newValue: any, oldValue: any) => void) | null
if (typeof propDef.observer === 'function') {
observer = propDef.observer
Expand Down Expand Up @@ -1265,8 +1282,7 @@ export class Behavior<
d = {
type,
optionalTypes,
value,
default: propDef.default,
defaultFn,
observer,
comparer,
reflectIdPrefix,
Expand All @@ -1275,14 +1291,14 @@ export class Behavior<
this._$propertyMap[name] = d
initValueFuncs.push({
name,
func: d.default === undefined ? () => simpleDeepCopy(d.value) : d.default,
func: initialValueFn,
})
}
this._$data.push(() => {
const ret: DataList = {}
for (let i = 0; i < initValueFuncs.length; i += 1) {
const { name, func } = initValueFuncs[i]!
ret[name] = safeCallback(`Property "${name}" Default`, func, null, [], is)
ret[name] = func()
}
return ret
})
Expand Down
84 changes: 37 additions & 47 deletions glass-easel/src/data_proxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ export const enum NormalizedPropertyType {
export type PropertyDefinition = {
type: NormalizedPropertyType
optionalTypes: NormalizedPropertyType[] | null
value: unknown
default: (() => unknown) | undefined
defaultFn: () => unknown
observer: ((newValue: unknown, oldValue: unknown) => void) | null
comparer: ((newValue: unknown, oldValue: unknown) => boolean) | null
reflectIdPrefix: boolean
Expand Down Expand Up @@ -76,13 +75,31 @@ export const shallowMerge = (dest: { [key: string]: unknown }, src: { [key: stri
}
}

export const getPropertyFallbackValue = (type: NormalizedPropertyType) => {
switch (type) {
case NormalizedPropertyType.String:
return ''
case NormalizedPropertyType.Number:
return 0
case NormalizedPropertyType.Boolean:
return false
case NormalizedPropertyType.Array:
return []
case NormalizedPropertyType.Function:
return function () {
/* empty */
}
default:
return null
}
}

export const normalizePropertyTypeShortHand = (propDef: unknown): PropertyDefinition | null => {
if (propDef === NormalizedPropertyType.String || propDef === String) {
return {
type: NormalizedPropertyType.String,
optionalTypes: null,
value: '',
default: undefined,
defaultFn: () => '',
observer: null,
comparer: null,
reflectIdPrefix: false,
Expand All @@ -92,8 +109,7 @@ export const normalizePropertyTypeShortHand = (propDef: unknown): PropertyDefini
return {
type: NormalizedPropertyType.Number,
optionalTypes: null,
value: 0,
default: undefined,
defaultFn: () => 0,
observer: null,
comparer: null,
reflectIdPrefix: false,
Expand All @@ -103,8 +119,7 @@ export const normalizePropertyTypeShortHand = (propDef: unknown): PropertyDefini
return {
type: NormalizedPropertyType.Boolean,
optionalTypes: null,
value: false,
default: undefined,
defaultFn: () => false,
observer: null,
comparer: null,
reflectIdPrefix: false,
Expand All @@ -114,8 +129,7 @@ export const normalizePropertyTypeShortHand = (propDef: unknown): PropertyDefini
return {
type: NormalizedPropertyType.Object,
optionalTypes: null,
value: null,
default: undefined,
defaultFn: () => null,
observer: null,
comparer: null,
reflectIdPrefix: false,
Expand All @@ -125,8 +139,7 @@ export const normalizePropertyTypeShortHand = (propDef: unknown): PropertyDefini
return {
type: NormalizedPropertyType.Array,
optionalTypes: null,
value: [],
default: undefined,
defaultFn: () => [],
observer: null,
comparer: null,
reflectIdPrefix: false,
Expand All @@ -136,10 +149,10 @@ export const normalizePropertyTypeShortHand = (propDef: unknown): PropertyDefini
return {
type: NormalizedPropertyType.Function,
optionalTypes: null,
value() {
/* empty */
},
default: undefined,
defaultFn: () =>
function () {
/* empty */
},
observer: null,
comparer: null,
reflectIdPrefix: false,
Expand All @@ -149,8 +162,7 @@ export const normalizePropertyTypeShortHand = (propDef: unknown): PropertyDefini
return {
type: NormalizedPropertyType.Any,
optionalTypes: null,
value: null,
default: undefined,
defaultFn: () => null,
observer: null,
comparer: null,
reflectIdPrefix: false,
Expand Down Expand Up @@ -188,20 +200,8 @@ export const convertValueToType = (
value: unknown,
propName: string,
prop: PropertyDefinition,
component: GeneralComponent | null,
): unknown => {
const type = prop.type
const defaultFn =
prop.default === undefined
? undefined
: () =>
safeCallback(
`Property "${propName}" Default`,
prop.default!,
null,
[],
component || undefined,
)
// try match optional types
const optionalTypes = prop.optionalTypes
if (optionalTypes) {
Expand All @@ -217,7 +217,7 @@ export const convertValueToType = (
triggerWarning(
`property "${propName}" received type-incompatible value: expected <String> but get null value. Used default value instead.`,
)
return defaultFn === undefined ? '' : defaultFn()
return prop.defaultFn()
}
if (typeof value === 'object') {
triggerWarning(
Expand All @@ -239,7 +239,7 @@ export const convertValueToType = (
`property "${propName}" received type-incompatible value: expected <Number> but got non-number value. Used default value instead.`,
)
}
return defaultFn === undefined ? 0 : defaultFn()
return prop.defaultFn()
}
// for boolean
if (type === NormalizedPropertyType.Boolean) {
Expand All @@ -251,31 +251,26 @@ export const convertValueToType = (
triggerWarning(
`property "${propName}" received type-incompatible value: expected <Array> but got non-array value. Used default value instead.`,
)
return defaultFn === undefined ? [] : defaultFn()
return prop.defaultFn()
}
// for object
if (type === NormalizedPropertyType.Object) {
if (typeof value === 'object') return value
triggerWarning(
`property "${propName}" received type-incompatible value: expected <Object> but got non-object value. Used default value instead.`,
)
return defaultFn === undefined ? null : defaultFn()
return prop.defaultFn()
}
// for function
if (type === NormalizedPropertyType.Function) {
if (typeof value === 'function') return value
triggerWarning(
`property "${propName}" received type-incompatible value: expected <Function> but got non-function value. Used default value instead.`,
)
// eslint-disable-next-line func-names
return defaultFn === undefined
? function () {
/* empty */
}
: defaultFn()
return prop.defaultFn()
}
// for any-typed, just return the value and avoid undefined
if (value === undefined) return defaultFn === undefined ? null : defaultFn()
if (value === undefined) return prop.defaultFn()
return value
}

Expand Down Expand Up @@ -727,12 +722,7 @@ export class DataGroup<
filteredData = oldData
} else {
// normal replace for properties
filteredData = convertValueToType(
newData,
propName,
prop,
this._$comp as GeneralComponent | null,
)
filteredData = convertValueToType(newData, propName, prop)
}

// if inner data is separated, update it
Expand Down
Loading
Loading