-
-
Notifications
You must be signed in to change notification settings - Fork 215
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
fix: #2960 type error #3066
base: master
Are you sure you want to change the base?
fix: #2960 type error #3066
Conversation
@Janpot If you want to achieve this goal, I suggest invoking the applyDomDiff after detecting the error. This way, we can ensure that the frontend has already received an error message |
@Janpot please review |
@@ -980,6 +980,7 @@ export default function AppProvider({ appUrl, children }: DomContextProps) { | |||
dispatch({ type: 'DOM_SAVED', savedDom: domToSave }); | |||
}) | |||
.catch((err) => { | |||
console.error(err.message); |
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.
Let's give users some context
console.error(err.message); | |
console.error(`Failed to save the dom: ${err.message}`); |
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.
How do you feel about the following
- when we detect that prettier errored, we show a confirmation dialog "prettier failed with error X, do you want to save without formatting".
- this sets a
ignoreFormattingErrors
to thestate
- add a
force: boolean
option toapplyDomDiff
which tries to prettify but ignores errors, we call it with:applyDomDiff(state.dom, { force: state.ignoreFormattingErrors })
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.
let me try
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.
when we detect that prettier errored, we show confirmation a dialog "prettier failed with error X, do you want to save without formatting".
how to display this dialog ? if a format error is detected, this dialog
will be always displayed
is there a strategy for this dialog?
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.
if a format error is detected, this dialog will be always displayed
is there a strategy for this dialog?
If a user confirms the dialog, then ignoreFormattingErrors
will get set in the state, and the next time applyDomDiff
is called it will get a force: true
value and shouldn't error on formatting errors.
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.
How about the ignoreFormattingErrors gets false? we don't do anything?
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.
We don't leave that option open. You either confirm or fix your setup. i.e. We say:
Failed to save because of an error in your formatter:
{error message}
Click Ignore to save without formatting, or "Retry" to try again.
#2960