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

Add isMDXComponent static class prop to generated classes #369

Merged
merged 11 commits into from
Feb 15, 2019
Merged

Add isMDXComponent static class prop to generated classes #369

merged 11 commits into from
Feb 15, 2019

Conversation

hugmanrique
Copy link
Contributor

There's currently no way to detect whether a class was generated by @mdx-js/mdx, and this PR adds a isMDXComponent() static class property to check whether the exported component was generated by mdx.

Closes #368

@vercel
Copy link

vercel bot commented Dec 28, 2018

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@hugmanrique
Copy link
Contributor Author

I'm sorry for all the commit spam, just wanted to try how the GitHub editor experience was 😝

@johno johno changed the title [WIP] Add isMDXComponent static class prop to generated classes Add isMDXComponent static class prop to generated classes Jan 24, 2019
Copy link
Member

@johno johno left a comment

Choose a reason for hiding this comment

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

🤙

@helfi92
Copy link

helfi92 commented Feb 13, 2019

Are there any blockers for this PR? I was wondering if we could have it merged. Many thanks.

@silvenon
Copy link
Contributor

silvenon commented Feb 13, 2019

Maybe checking displayName works?

import ChangeLog from './changelog.mdx'

ChangeLog.displayName === 'MDXContent'
// or
ChangeLog.displayName.includes('MDX')

@hugmanrique
Copy link
Contributor Author

This sadly didn't work for uglified builds where class names get changed.

Copy link
Member

@johno johno left a comment

Choose a reason for hiding this comment

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

I think it might make more sense to make this a boolean rather than a function so folks don't have to do Component.isMDXComponent && Component.isMDXComponent().

packages/mdx/mdx-hast-to-jsx.js Outdated Show resolved Hide resolved
packages/mdx/test/index.test.js Outdated Show resolved Hide resolved
packages/mdx/test/index.test.js Outdated Show resolved Hide resolved
@silvenon
Copy link
Contributor

This sadly didn't work for uglified builds where class names get changed.

Are you sure you didn't accidentally try name instead of displayName? AFAIK displayName stays as-is after uglifying because it's a property value.

And thanks for your work on this PR, I'm just curious. 😄

Co-Authored-By: hugmanrique <hugmanrique@users.noreply.github.com>
@vercel vercel bot temporarily deployed to staging February 14, 2019 21:21 Inactive
Co-Authored-By: hugmanrique <hugmanrique@users.noreply.github.com>
@vercel vercel bot temporarily deployed to staging February 14, 2019 21:22 Inactive
@johno
Copy link
Member

johno commented Feb 15, 2019

Thanks @hugmanrique!

@johno johno merged commit 589f293 into mdx-js:master Feb 15, 2019
johno pushed a commit that referenced this pull request Feb 18, 2019
* Add `isMDXElement` static class prop to generated classes

Adds the behavior discussed in #368

* s/element/component

* Add isMDXComponent test

* Define static class prop the old way

* Fix prettier error

* Revert new line removal

* Fix prettier error

* Add function to snapshot test

* Missed a ;

* Change isMDXComponent type to bool

Co-Authored-By: hugmanrique <hugmanrique@users.noreply.github.com>

* Apply suggestions from code review

Co-Authored-By: hugmanrique <hugmanrique@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants