sunmao-ui
sunmao-ui copied to clipboard
runtime type consistency in expression evaluation.
https://github.com/webzard-io/sunmao-ui/blob/5fe5163733/packages/runtime/src/services/stateStore.ts#L106-L108
Currently, we handle expression evaluation errors with error logs but will return undefined as the result.
This makes components can not predict the types of props in runtime. For example, a component may define a props foo
with type number
which is required, but if the evaluation throws an error, the component may get undefined
as the foo
props value.
Possible solution: do not render components in ImplementWrapper
when expression evaluation failed.
I think not-rendering components as a fallback could be too over. How about passing a default value according to the type, like 0
to number, ''
to string, false
to boolean.
@tanbowensg
For example, we have a component <Bill />
with a props payTheBill
. when payTheBill
is true
, the component will send a request and pay the bill, otherwise, the component will do nothing.
It's quite ok if an application builder writes a wrong expression and accidentally fires the payment request because it's a user error.
But it's not ok if an application builder writes an invalid expression and we fall back to true
or false
, which accidentally fires the request.
Falling back to any type-right value is dangerous because we do not know the semantics of the props are "pay the bill" or "do not pay the bill".
If we add defaultValue
in properties spec, can we solve this problem? We can always fallback to the safe value that is set by the component developer.
@tanbowensg
I think this is an issue of soundness.
What's the expected behavior when a user writes a wrong expression like {{ const a }}
and passes the schema to sunmao-ui's runtime?
Responses I've thought about:
- sunmao-ui's runtime crash and throw the original expression error to the global scope
- sunmao-ui's runtime catch the error and render a mask UI to display the error
- sunmao-ui's runtime catch the error in component level and render a mask UI to display the error on top of the corresponding component
- sunmao-ui's validator catch the error and show a wrapped error message
- sunmao-ui's runtime catch the error and fallback to
defaultValue
, then log the error to console
Of course, there may be some different ways to handle this in dev/production environments or in runtime/editor scope, but I think we can start from the specific one.
Another problem, property value will change after toggle property from normal mode to JS mode.
I think the runtime should catch the error and fall back to the property's defaultValue
first. If the property has no defaultValue
, then pass the undefined
as the component property value.
Maybe it would cause the application to crash, but I think that is fine. Just like running the wrong JavaScript codes in v8 and the application would be crash too.
And in the editor, when the expression fails to eval or the value is wrong, the editor should display the error in the properties form and the component wrapper like the image below.
the editor should display the error in the properties form and the component wrapper like the image below.
Yanzhen worries about it will cause silent bug if we fallback to default value. Maybe displaying error message maybe could avoid this problem.
As to the default value, should we defind default value in spec?
Yanzhen worries about it will cause silent bug if we fallback to default value. Maybe displaying error message maybe could avoid this problem.
I think the component developers should handle the property's value could be undefined
condition caused by a wrong expression or empty input and consider whether to provide a default value for the property or throw the errors.
Displaying the error messages could help the Sunmao users fix some expression errors but can't avoid them totally so I think we should handle the wrong expression in some ways.
As to the default value, should we defind default value in spec?
Yes. When switching the JS mode, the expression string is hard to display by the other widgets. Thus, I think maybe set the property to the default value when switching the JS mode.
In addition, I think we should also remain the exampleProperties
in the spec because the defaultValue
is used to init or reset or be a fallback and the exampleProperties
is used to show the base usages of components when inserting the components.
I think the component developers should handle the property's value could be undefined condition caused by a wrong expression or empty input and consider whether to provide a default value for the property or throw the errors.
I'm not sure about this. If the component developers declared the props like { value: string }
both in typescript and JSON schema, it can be a big flaw that they need to check whether value
is undefined and write some fallback code. Especially when they are wrapping existing components to sunmao-ui components.
Maybe it would cause the application to crash, but I think that is fine. Just like running the wrong JavaScript codes in v8 and the application would be crash too.
I think this is about when to crash.
For example, when looking at the following code:
function component(props) {
const UI = doSomething(props)
return render(UI)
}
function main() {
const props = evalProps(store)
return component(props)
}
main()
If there is something happened in the evalProps
function, it feels weird the component
function can be executed.