-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: run formatter/prettier on launchpad's generated code in Editor #975
feat: run formatter/prettier on launchpad's generated code in Editor #975
Conversation
@@ -151,8 +151,9 @@ const Editor: React.FC = (): ReactElement => { | |||
} | |||
|
|||
const codeResponse = await generateCode(wizardContractFilter, wizardMethods, wizardEvents); | |||
setIndexingCode(codeResponse.jsCode); | |||
setSchema(codeResponse.sqlCode); | |||
const { validatedCode, validatedSchema } = reformatAll(codeResponse.jsCode, codeResponse.sqlCode); |
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.
Since we handle and error throwing and check for properties within generateCode, we can safely assume the response is correct at this point so we can call for formatting
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 use effect seems to be triggered by nothing? I'm confused when this runs. I'm mainly worried about the reformat changing the code suddenly for the user, which may be confusing.
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.
Question here as well. I noticed that if I go to an indexer, the diff view compares the default code (the one where we do context.set on the block height) with my current code. Not my code when I opened the page, and my current code. Could you investigate that? Seeing as this PR is non-essential and small, it might be better to do that here.
Also, this PR is titled something related to formatting, but no files added to this PR have any formatting changes. Are you sure you added them? Otherwise, if the formatting was already correct, it should be re-titled to cover whatever DID change.
@@ -151,8 +151,9 @@ const Editor: React.FC = (): ReactElement => { | |||
} | |||
|
|||
const codeResponse = await generateCode(wizardContractFilter, wizardMethods, wizardEvents); | |||
setIndexingCode(codeResponse.jsCode); | |||
setSchema(codeResponse.sqlCode); | |||
const { validatedCode, validatedSchema } = reformatAll(codeResponse.jsCode, codeResponse.sqlCode); |
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 use effect seems to be triggered by nothing? I'm confused when this runs. I'm mainly worried about the reformat changing the code suddenly for the user, which may be confusing.
No description provided.