-
-
Notifications
You must be signed in to change notification settings - Fork 273
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.
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
format IDE code #2674
format IDE code #2674
Conversation
let newValue = input; | ||
|
||
if ((input as JsExpressionAttrValue)?.$$jsExpression) { | ||
const jsExpression = await tryFormatExpression( | ||
(input as JsExpressionAttrValue).$$jsExpression, | ||
data!, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the backend call to getPrettierConfig
fails, or is still loading, data
will be undefined
. So using !
is not the right solution as that is only reserved for when data
is guaranteed to be defined by the programmer.
Instead, show a warning to the user that explains that the prettier config couldn't be loaded and therefore the code won't be formatted when saving.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the backend call to
getPrettierConfig
fails, or is still loading,data
will beundefined
. So using!
is not the right solution as that is only reserved for whendata
is guaranteed to be defined by the programmer.Instead, show a warning to the user that explains that the prettier config couldn't be loaded and therefore the code won't be formatted when saving.
what's type of warning for user?
such as https://mui.com/material-ui/react-alert/?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there component that i can directly invoke ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An Alert could work. I'd have to see what it looks like. For me I think it's important it's inline, close enough to the save button (i.e. no popup)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this error only happens without a network I think
There could be a problem while prettier tries to access the file system, or a bug in our code, or in prettier, or our bundler could have transformed the code wrong, or it could be running on an incompatible node.js runtime that we're unaware about, a solar storm flipping a bit,... anything really 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There could be a problem while prettier tries to access the file system, or a bug in our code, or in prettier, or our bundler could have transformed the code wrong, or it could be running on an incompatible node.js runtime that we're unaware about, a solar storm flipping a bit,... anything really
I see.
do these errors need to be passed to front-end, if back-end occurs error ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Janpot
please make a final check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JerryWu1234 useQuery
is a wrapper around react-query
. Next to the data
property it will return an error
property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice point out.
before I thought we need to add a try catch
and then pass to front end. lol
I will do it late
…Wu1234/mui-toolpad into format_front_end_code_2585
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, thanks!
#2585