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.

Sign up for GitHub

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

Jump to bottom

Export the MDXContent component after its instantiated #1024

Closed
wants to merge 1 commit into from
Closed

Export the MDXContent component after its instantiated #1024

wants to merge 1 commit into from

Conversation

johno
Copy link
Member

@johno johno commented Apr 22, 2020

This caused a warning in Firefox because the
isMDXContent assignment was after the export
which caused it to result after the component
return in the transpiled output.

Related: gatsbyjs/gatsby#23171

Co-authored-by: Laurie Barth [email protected]

@johno
Export the MDXContent component after its instantiated
506885b 
This caused a warning in Firefox because the
isMDXContent assignment was after the export
which caused it to result after the component
return in the transpiled output.

Related: gatsbyjs/gatsby#23171

Co-authored-by: Laurie Barth <[email protected]>
@johno johno added 🐛 type/bug This is a problem 🏡 area/internal This affects the hidden internals 🏗 area/tools This affects tooling 👶 semver/patch This is a backwards-compatible fix 🌐 platform/browser This affects browsers labels Apr 22, 2020
@vercel
Copy link

vercel bot commented Apr 22, 2020 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click on the icon next to each commit.

@johno johno mentioned this pull request Apr 22, 2020
Closed
silvenon
silvenon suggested changes Apr 24, 2020
View reviewed changes
Copy link
Contributor

@silvenon silvenon left a comment

Choose a reason for hiding this comment

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

Other than the diff problem, looks good! 👍

packages/mdx/test/index.test.js
MDXContent.isMDXComponent = true;

export default MDXContent"
`)
Copy link
Contributor

@silvenon silvenon Apr 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

😅 I think you can solve this noisy diff by simply indenting the content of toMatchInlineSnapshot. As a separate change we should probably start using toMatchSnapshot because the content is large.

@mohsenkhanpour
Copy link

Any update on this?

@wooorm
Copy link
Member

wooorm commented Nov 13, 2020

@johno Is there anything blocking this? What can I do to help?

@wooorm
Copy link
Member

wooorm commented Dec 17, 2020

Solved by #1382 btw

@wooorm
Copy link
Member

wooorm commented Dec 18, 2020

Hi all! I’m going to close this as it landed on the main, the (now default) branch we’re using to gear up to the MDX@2 release.

The reason is so that it’s more clear which issues still need to be solved already and don’t require further work.

Expect another next release soonish, which might include it, or maybe it’s already released as a beta!

For more info, see #1041.

@wooorm wooorm closed this Dec 18, 2020
@wooorm wooorm deleted the export-after-function branch December 18, 2020 16:42
@wooorm wooorm added the 💪 phase/solved Post is done label Dec 25, 2021
@jonohewitt jonohewitt mentioned this pull request Feb 16, 2022
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@silvenon silvenon silvenon requested changes

Assignees
No one assigned
Labels
🏡 area/internal This affects the hidden internals 🏗 area/tools This affects tooling 💪 phase/solved Post is done 🌐 platform/browser This affects browsers 👶 semver/patch This is a backwards-compatible fix 🐛 type/bug This is a problem
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@johno @mohsenkhanpour @wooorm @silvenon