Skip to content
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

chore(gatsby-plugin-mdx): refactor mdx-loader async code #25790

Closed
wants to merge 2 commits into from

Conversation

pvdz
Copy link
Contributor

@pvdz pvdz commented Jul 16, 2020

This function was not optimally using the syntactic sugar of async functions. It was manually creating and returning a promise, including dealing with exceptions. Instead, the async keyword takes care of all of that for you implicitly. So this simplifies the function a little bit.

Suggest to review without whitespace.

This function was not optimally using the syntactic sugar of `async` functions. It was manually creating and returning a promise, including dealing with exceptions. Instead, the `async` keyword takes care of all of that for you implicitly. So this simplifies the function a little bit.
@pvdz pvdz requested a review from a team as a code owner July 16, 2020 13:14
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jul 16, 2020
@pvdz pvdz added topic: MDX and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Jul 16, 2020
@laurieontech
Copy link
Contributor

Should this merge into master or merge into #25757?

@pvdz
Copy link
Contributor Author

pvdz commented Jul 16, 2020

#25757 is likely to merge to master long after this one so I'll rebase that PR to master once it does and resolve the inevitable merge conflict.

@pvdz
Copy link
Contributor Author

pvdz commented Jul 16, 2020

Once I fix the tests ... 🤔

@gatsby-cloud-staging
Copy link

Your pull request can be previewed in Gatsby Cloud: https://build-4ea45a4c-3d40-43d2-9693-5aefb40130a1.staging-previews.gtsb.io

@pvdz
Copy link
Contributor Author

pvdz commented Jul 17, 2020

I'm going to need more time to validate this is not going to break webpack so I'm going to put it in draft mode and return to it after my vacation,

@pvdz pvdz marked this pull request as draft July 17, 2020 10:00
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're not sure you can always do this:

module.exports = function loader(content) {
  const callback = this.async();

  mdxLoader
      .call(this, content)
      .then(args => callback(null, args), err => callback(err));
}

async function mdxLoader(content) {

@pvdz pvdz closed this Sep 8, 2020
@LekoArts LekoArts deleted the mdx-refactor-mdxloader branch November 9, 2020 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants