From 41c82673840852d654340e6388b95eda2c3e5bd6 Mon Sep 17 00:00:00 2001 From: Bowen Tan Date: Tue, 26 Jul 2022 14:25:29 +0800 Subject: [PATCH 1/6] fix(validator): validator cannot validate nested object in expression --- .../__tests__/validator/component.spec.ts | 7 ++++ packages/editor/__tests__/validator/mock.ts | 41 +++++++++++++++++++ .../src/validator/rules/PropertiesRules.ts | 3 +- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/packages/editor/__tests__/validator/component.spec.ts b/packages/editor/__tests__/validator/component.spec.ts index ab0002a3..b9394160 100644 --- a/packages/editor/__tests__/validator/component.spec.ts +++ b/packages/editor/__tests__/validator/component.spec.ts @@ -5,6 +5,7 @@ import { UseDependencyInExpressionSchema, LocalVariableInIIFEExpressionSchema, DynamicStateTraitAnyTypeSchema, + NestedObjectExpressionSchema, } from './mock'; import { SchemaValidator } from '../../src/validator'; import { registry } from '../services'; @@ -68,5 +69,11 @@ describe('Validate component', () => { ); expect(_result.length).toBe(0); }); + it('allow use nested object value in expression', () => { + const _result = schemaValidator.validate( + new AppModel(NestedObjectExpressionSchema, registry) + ); + expect(_result.length).toBe(0); + }); }); }); diff --git a/packages/editor/__tests__/validator/mock.ts b/packages/editor/__tests__/validator/mock.ts index 4b25eed5..2b62a848 100644 --- a/packages/editor/__tests__/validator/mock.ts +++ b/packages/editor/__tests__/validator/mock.ts @@ -414,3 +414,44 @@ export const DynamicStateTraitSchema: ComponentSchema[] = [ traits: [], }, ]; + +export const NestedObjectExpressionSchema: ComponentSchema[] = [ + { + id: 'api0', + type: 'core/v1/dummy', + properties: {}, + traits: [ + { + type: 'core/v1/fetch', + properties: { + url: '', + method: 'get', + lazy: false, + disabled: false, + headers: {}, + body: {}, + bodyType: 'json', + onComplete: [ + { + componentId: '', + method: { + name: '', + }, + }, + ], + onError: [ + { + componentId: '', + method: { + name: '', + parameters: {}, + }, + // should not warn api0.fetch.code + disabled: '{{ api0.fetch.code !== 401 }}', + }, + ], + }, + }, + ], + }, +]; diff --git a/packages/editor/src/validator/rules/PropertiesRules.ts b/packages/editor/src/validator/rules/PropertiesRules.ts index 69929d1d..e030e113 100644 --- a/packages/editor/src/validator/rules/PropertiesRules.ts +++ b/packages/editor/src/validator/rules/PropertiesRules.ts @@ -77,7 +77,7 @@ class ExpressionValidatorRule implements PropertiesValidatorRule { private checkObjHasPath(obj: Record, path: string) { const arr = path.split('.'); - const curr = obj; + let curr = obj; for (const key of arr) { const value = curr[key]; if (value === undefined) { @@ -86,6 +86,7 @@ class ExpressionValidatorRule implements PropertiesValidatorRule { // if meet AnyTypePlaceholder, return true and skip return true; } + curr = value; } return true; } From 7f2fdd5f6373281a7cb2bdd9d57e8a3d2d765377 Mon Sep 17 00:00:00 2001 From: Bowen Tan Date: Tue, 26 Jul 2022 14:32:31 +0800 Subject: [PATCH 2/6] fix(validator): it should not warn variables declared in iife in expression --- packages/editor/__tests__/model/fieldModel.spec.ts | 6 ++++++ packages/editor/src/AppModel/FieldModel.ts | 6 +++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/editor/__tests__/model/fieldModel.spec.ts b/packages/editor/__tests__/model/fieldModel.spec.ts index 161ad68a..decc956c 100644 --- a/packages/editor/__tests__/model/fieldModel.spec.ts +++ b/packages/editor/__tests__/model/fieldModel.spec.ts @@ -71,6 +71,12 @@ describe('Field test', () => { expect(field.refComponentInfos).toEqual({}); }); + it('should not count variables declared in iife in refs', () => { + const field = new FieldModel('{{(function() {const foo = "bar"})() }}'); + expect(field.isDynamic).toEqual(true); + expect(field.refComponentInfos).toEqual({}); + }); + it('get value by path', () => { const field = new FieldModel({ foo: [{}, { bar: { baz: 'Hello, world!' } }] }); expect(field.getPropertyByPath('foo.1.bar.baz')?.getValue()).toEqual('Hello, world!'); diff --git a/packages/editor/src/AppModel/FieldModel.ts b/packages/editor/src/AppModel/FieldModel.ts index 3dcffda8..b7c207eb 100644 --- a/packages/editor/src/AppModel/FieldModel.ts +++ b/packages/editor/src/AppModel/FieldModel.ts @@ -20,6 +20,7 @@ import escodegen from 'escodegen'; import { JSONSchema7 } from 'json-schema'; export type FunctionNode = ASTNode & { params: ASTNode[] }; +export type DeclaratorNode = ASTNode & { id: ASTNode }; export class FieldModel implements IFieldModel { isDynamic = false; refComponentInfos: Record = {}; @@ -161,7 +162,7 @@ export class FieldModel implements IFieldModel { this.astNodes[exp] = node as ASTNode; - // These are varirables of iife, they should be count in refs. + // These are variables of iife, they should be count in refs. let localVariables: ASTNode[] = []; simpleWalk(node, { @@ -200,6 +201,9 @@ export class FieldModel implements IFieldModel { default: } }, + VariableDeclarator: declarator => { + localVariables.push((declarator as DeclaratorNode).id); + }, }); // remove localVariables from refs From d21e1526f60a1f7e6e75f5462760d82eb087e2a8 Mon Sep 17 00:00:00 2001 From: Bowen Tan Date: Tue, 26 Jul 2022 18:07:42 +0800 Subject: [PATCH 3/6] fix(validator): add AnyTypePlaceholder for nester object --- packages/shared/__tests__/spec.spec.ts | 8 ++++++ packages/shared/src/utils/spec.ts | 38 +++++++++++++++++--------- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/packages/shared/__tests__/spec.spec.ts b/packages/shared/__tests__/spec.spec.ts index 0efaacc3..3f57bc10 100644 --- a/packages/shared/__tests__/spec.spec.ts +++ b/packages/shared/__tests__/spec.spec.ts @@ -93,5 +93,13 @@ describe('generateDefaultValueFromSpec function', () => { it('can parse any type', () => { expect(generateDefaultValueFromSpec({})).toEqual(''); expect(generateDefaultValueFromSpec({}, true)).toEqual(AnyTypePlaceholder); + expect( + ( + generateDefaultValueFromSpec( + Type.Object({ foo: Type.Object({ bar: Type.Any() }) }), + true + ) as any + ).foo.bar + ).toEqual(AnyTypePlaceholder); }); }); diff --git a/packages/shared/src/utils/spec.ts b/packages/shared/src/utils/spec.ts index 84865571..18d26be0 100644 --- a/packages/shared/src/utils/spec.ts +++ b/packages/shared/src/utils/spec.ts @@ -23,20 +23,31 @@ export function StringUnion(values: [...T], options?: any) { ); } -function getArray(items: JSONSchema7Definition[]): JSONSchema7Type[] { +function getArray( + items: JSONSchema7Definition[], + returnPlaceholderForAny = false +): JSONSchema7Type[] { return items.map(item => - isJSONSchema(item) ? generateDefaultValueFromSpec(item) : null + isJSONSchema(item) + ? generateDefaultValueFromSpec(item, returnPlaceholderForAny) + : null ); } -function getObject(spec: JSONSchema7): JSONSchema7Object { +function getObject( + spec: JSONSchema7, + returnPlaceholderForAny = false +): JSONSchema7Object { const obj: JSONSchema7Object = {}; if (spec.allOf && spec.allOf.length > 0) { - return (getArray(spec.allOf) as JSONSchema7Object[]).reduce((prev, cur) => { - prev = Object.assign(prev, cur); - return prev; - }, obj); + return (getArray(spec.allOf, returnPlaceholderForAny) as JSONSchema7Object[]).reduce( + (prev, cur) => { + prev = Object.assign(prev, cur); + return prev; + }, + obj + ); } for (const key in spec.properties) { @@ -44,7 +55,7 @@ function getObject(spec: JSONSchema7): JSONSchema7Object { if (typeof subSpec === 'boolean') { obj[key] = null; } else if (subSpec) { - obj[key] = generateDefaultValueFromSpec(subSpec); + obj[key] = generateDefaultValueFromSpec(subSpec, returnPlaceholderForAny); } } return obj; @@ -58,10 +69,11 @@ export function generateDefaultValueFromSpec( if ((spec.anyOf && spec.anyOf!.length > 0) || (spec.oneOf && spec.oneOf.length > 0)) { const subSpec = (spec.anyOf! || spec.oneOf)[0]; if (typeof subSpec === 'boolean') return null; - return generateDefaultValueFromSpec(subSpec); + return generateDefaultValueFromSpec(subSpec, returnPlaceholderForAny); } // It is any type + console.log('any', returnPlaceholderForAny); if (returnPlaceholderForAny) { return AnyTypePlaceholder; } @@ -77,7 +89,7 @@ export function generateDefaultValueFromSpec( const subSpec = { type: spec.type[0], } as JSONSchema7; - return generateDefaultValueFromSpec(subSpec); + return generateDefaultValueFromSpec(subSpec, returnPlaceholderForAny); } case spec.type === 'string': if (spec.enum && spec.enum.length > 0) { @@ -90,16 +102,16 @@ export function generateDefaultValueFromSpec( case spec.type === 'array': return spec.items ? Array.isArray(spec.items) - ? getArray(spec.items) + ? getArray(spec.items, returnPlaceholderForAny) : isJSONSchema(spec.items) - ? [generateDefaultValueFromSpec(spec.items)] + ? [generateDefaultValueFromSpec(spec.items, returnPlaceholderForAny)] : null : []; case spec.type === 'number': case spec.type === 'integer': return 0; case spec.type === 'object': - return getObject(spec); + return getObject(spec, returnPlaceholderForAny); case spec.type === 'null': return null; default: From cf62b867ba6104f36be839d25f916c8539f7798e Mon Sep 17 00:00:00 2001 From: Bowen Tan Date: Wed, 27 Jul 2022 10:31:25 +0800 Subject: [PATCH 4/6] fix(validator): allow using question mask in expression --- .../editor/__tests__/model/fieldModel.spec.ts | 13 ++++++-- packages/editor/src/AppModel/FieldModel.ts | 30 ++++++++++++++----- packages/shared/src/utils/spec.ts | 1 - 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/packages/editor/__tests__/model/fieldModel.spec.ts b/packages/editor/__tests__/model/fieldModel.spec.ts index decc956c..7be90a00 100644 --- a/packages/editor/__tests__/model/fieldModel.spec.ts +++ b/packages/editor/__tests__/model/fieldModel.spec.ts @@ -20,12 +20,21 @@ describe('Field test', () => { 'value', ]); expect(field.refComponentInfos['list' as ComponentId].refProperties).toEqual([ - '[0]', - '[0].text', + '0', + '0.text', ]); expect(field.rawValue).toEqual('{{input.value}} + {{list[0].text}}'); }); + it('allow using question mask in expression', () => { + const field = new FieldModel('{{api.fetch?.data }}'); + expect(field.isDynamic).toEqual(true); + expect(field.refComponentInfos['api' as ComponentId].refProperties).toEqual([ + 'fetch', + 'fetch.data', + ]); + }); + it('parse inline variable in expression', () => { const field = new FieldModel('{{ [].length }}'); expect(field.isDynamic).toEqual(true); diff --git a/packages/editor/src/AppModel/FieldModel.ts b/packages/editor/src/AppModel/FieldModel.ts index b7c207eb..7b0718cb 100644 --- a/packages/editor/src/AppModel/FieldModel.ts +++ b/packages/editor/src/AppModel/FieldModel.ts @@ -21,6 +21,11 @@ import { JSONSchema7 } from 'json-schema'; export type FunctionNode = ASTNode & { params: ASTNode[] }; export type DeclaratorNode = ASTNode & { id: ASTNode }; +export type LiteralNode = ASTNode & { raw: string }; +export type ExpressionNode = ASTNode & { + object: ExpressionNode; + property: ExpressionNode | LiteralNode; +}; export class FieldModel implements IFieldModel { isDynamic = false; refComponentInfos: Record = {}; @@ -191,12 +196,9 @@ export class FieldModel implements IFieldModel { break; case 'MemberExpression': - const str = exp.slice(expressionNode.start, expressionNode.end); - let path = str.replace(lastIdentifier, ''); - if (path.startsWith('.')) { - path = path.slice(1, path.length); - } - this.refComponentInfos[lastIdentifier]?.refProperties.push(path); + this.refComponentInfos[lastIdentifier]?.refProperties.push( + this.genPathFromMemberExpressionNode(expressionNode as ExpressionNode) + ); break; default: } @@ -205,7 +207,6 @@ export class FieldModel implements IFieldModel { localVariables.push((declarator as DeclaratorNode).id); }, }); - // remove localVariables from refs for (const key in this.refComponentInfos) { if (localVariables.some(({ name }) => key === name)) { @@ -215,6 +216,21 @@ export class FieldModel implements IFieldModel { }); } + private genPathFromMemberExpressionNode(expNode: ExpressionNode) { + const path: string[] = []; + function travel(node: ExpressionNode) { + path.unshift( + node.property?.name || (node.property as LiteralNode)?.raw || node.name + ); + if (node.object) { + travel(node.object); + } + } + + travel(expNode); + return path.slice(1).join('.'); + } + private onReferenceIdChange({ oldId, newId }: AppModelEventType['idChange']) { if (!this.componentModel) { return; diff --git a/packages/shared/src/utils/spec.ts b/packages/shared/src/utils/spec.ts index 18d26be0..b459c499 100644 --- a/packages/shared/src/utils/spec.ts +++ b/packages/shared/src/utils/spec.ts @@ -73,7 +73,6 @@ export function generateDefaultValueFromSpec( } // It is any type - console.log('any', returnPlaceholderForAny); if (returnPlaceholderForAny) { return AnyTypePlaceholder; } From 85d653381f1fe3a167bdd38e1e6d3d32c4ec66d1 Mon Sep 17 00:00:00 2001 From: Bowen Tan Date: Wed, 27 Jul 2022 17:37:03 +0800 Subject: [PATCH 5/6] fix(validator): validator will crash when it fails to find util method --- packages/editor/src/validator/rules/TraitRules.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/editor/src/validator/rules/TraitRules.ts b/packages/editor/src/validator/rules/TraitRules.ts index d62cb7ad..62329e8a 100644 --- a/packages/editor/src/validator/rules/TraitRules.ts +++ b/packages/editor/src/validator/rules/TraitRules.ts @@ -94,6 +94,7 @@ class EventHandlerValidatorRule implements TraitValidatorRule { traitType: trait?.type, traitIndex, }); + return results; }); } } else { From d9671e6515b872ea6775e030571595e33aa0b67c Mon Sep 17 00:00:00 2001 From: Bowen Tan Date: Wed, 27 Jul 2022 17:58:40 +0800 Subject: [PATCH 6/6] fix(validator): if a object json schema does not have specific properties, treat it as any type --- packages/shared/src/utils/spec.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/shared/src/utils/spec.ts b/packages/shared/src/utils/spec.ts index b459c499..c8c9a408 100644 --- a/packages/shared/src/utils/spec.ts +++ b/packages/shared/src/utils/spec.ts @@ -37,7 +37,7 @@ function getArray( function getObject( spec: JSONSchema7, returnPlaceholderForAny = false -): JSONSchema7Object { +): JSONSchema7Object | string { const obj: JSONSchema7Object = {}; if (spec.allOf && spec.allOf.length > 0) { @@ -50,6 +50,15 @@ function getObject( ); } + // if not specific property, treat it as any type + if (!spec.properties) { + if (returnPlaceholderForAny) { + return AnyTypePlaceholder; + } + + return {}; + } + for (const key in spec.properties) { const subSpec = spec.properties?.[key]; if (typeof subSpec === 'boolean') { @@ -65,6 +74,7 @@ export function generateDefaultValueFromSpec( spec: JSONSchema7, returnPlaceholderForAny = false ): JSONSchema7Type { + console.log(spec); if (!spec.type) { if ((spec.anyOf && spec.anyOf!.length > 0) || (spec.oneOf && spec.oneOf.length > 0)) { const subSpec = (spec.anyOf! || spec.oneOf)[0];