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
Updates in line with inline updates #762
Updates in line with inline updates #762
Conversation
@@ -32,8 +32,8 @@ export const pageQuery = graphql\` | |||
\` | |||
` | |||
|
|||
export const ERROR_MISSING_MDX_RAW_MARKDOWN = | |||
'useMdxForm(mdx) Required attribute `rawMarkdownBody` was not found on `mdx` node.' + | |||
export const ERROR_MISSING_MDX_RAW_MDX = |
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.
perhaps this should be ERROR_MISSING_MDX_RAW_MDX_BODY
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.
What about ERROR_MISSING_RAW_MDX_BODY
?
yaml.dump(mdx.rawFrontmatter) + | ||
'---\n' + | ||
(mdx.rawMarkdownBody || '') | ||
'---\n' + yaml.dump(mdx.rawFrontmatter) + '---\n' + (mdx.rawMdxBody || '') |
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.
Template literal here might be easier to read
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.
Yeah, I think you're right, this is a bit hard to read 😂
I vote for template literal
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.
Template literals didn't work out because of indentation. Prettier wanted this to be a one liner so I changed it to an array so the formatting is made slightly more obvious.
@@ -53,7 +53,7 @@ export function useMdxForm( | |||
return [mdx, null] | |||
} | |||
|
|||
validateMdx(mdx) | |||
validateMdxExists(mdx) |
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.
rename to checkMDXExists
?
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.
Sounds good to me!
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.
One of the most confusing PR titles I've ever read, but the content is clear as day 👍
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.
Looks great! Nice work @PaulBunker 🎉🌮
I left comments, most of which were in agreement with the changes you suggested.
@@ -32,8 +32,8 @@ export const pageQuery = graphql\` | |||
\` | |||
` | |||
|
|||
export const ERROR_MISSING_MDX_RAW_MARKDOWN = | |||
'useMdxForm(mdx) Required attribute `rawMarkdownBody` was not found on `mdx` node.' + | |||
export const ERROR_MISSING_MDX_RAW_MDX = |
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.
What about ERROR_MISSING_RAW_MDX_BODY
?
yaml.dump(mdx.rawFrontmatter) + | ||
'---\n' + | ||
(mdx.rawMarkdownBody || '') | ||
'---\n' + yaml.dump(mdx.rawFrontmatter) + '---\n' + (mdx.rawMdxBody || '') |
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.
Yeah, I think you're right, this is a bit hard to read 😂
I vote for template literal
@@ -53,7 +53,7 @@ export function useMdxForm( | |||
return [mdx, null] | |||
} | |||
|
|||
validateMdx(mdx) | |||
validateMdxExists(mdx) |
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.
Sounds good to me!
Bad import in
|
Yes I noticed that last night, Confused why it built for me while I was doing this. Fixed now anyway. |
I've updated this branch with changes that were made in the remark plugin
I've also changed references to 'markdown' because mdx is not markdown and will break if it isn't valid.
Still working on a fix for validation