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/spectacle mdx loader addition #1228

Merged
merged 24 commits into from Feb 25, 2023

Conversation

paulmarsicloud
Copy link
Contributor

Description

First pass and attempt at bringing in spectacle-mdx-loader into the monorepo. Took onboard @ryan-roemer's advice to remove all devDeps and scripts and focus on getting lint and prettier working as a start. Moved prop-types and spectacle-mdx-loader into top-level package.json so that spectacle-mdx-loader could be called directly from examples/mdx/webpack.config.js.

In a good spot now for a second pair of eyes to take a look through 👍

Fixes #1164

Type of Change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How Has This Been Tested?

Checklist: (Feel free to delete this section upon completion)

  • I have included a changeset if this change will require a version change to one of the packages.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have run pnpm run check:ci and all checks pass
  • I have added tests that prove my fix is effective or that my feature works
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

examples/mdx/index.js Outdated Show resolved Hide resolved
examples/mdx/slides.mdx Outdated Show resolved Hide resolved
examples/mdx/test-component.js Show resolved Hide resolved
examples/mdx/slides.mdx Outdated Show resolved Hide resolved
examples/mdx/webpack.config.js Outdated Show resolved Hide resolved
packages/spectacle-mdx-loader/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/spectacle-mdx-loader/LICENSE.txt Show resolved Hide resolved
packages/spectacle-mdx-loader/package.json Show resolved Hide resolved
packages/spectacle-mdx-loader/src/helpers.js Show resolved Hide resolved
packages/spectacle-mdx-loader/README.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@ryan-roemer
Copy link
Member

@paulmarsicloud I've finished my cleanup of examples configs and dependencies. Mind double checking my commits?

@carlos-kelly @gksander @fritz-c I've noticed a discrepancy in code blocks in MD vs. MDX.

For our MD example with pnpm start:md for the code page we have:

Screen Shot 2022-09-21 at 1 50 03 PM

For our MDX example with pnpm start:mdx for the code page with exact same sample we have:

Screen Shot 2022-09-21 at 1 50 08 PM

Any ideas here? Thanks!

@ryan-roemer
Copy link
Member

Offhand, now that I'm looking at things looks like our Markdown component does a lot more than mdxComponentMap does alone -- and we recommend in our guide to just use the mdxComponentMap for MDX decks.

@ryan-roemer
Copy link
Member

Per slack discussion, we're not blocking on this and I've filed #1232 to capture.

Copy link
Member

@ryan-roemer ryan-roemer left a comment

Choose a reason for hiding this comment

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

Great work Paul! 🎉

@paulmarsicloud
Copy link
Contributor Author

@paulmarsicloud I've finished my cleanup of examples configs and dependencies. Mind double checking my commits?

Commits look good and works proper on my end!

@ryan-roemer
Copy link
Member

@paulmarsicloud We're missing a build task for examples/mdx which I'm going to add (and to CI)

@fritz-c fritz-c dismissed their stale review September 22, 2022 15:53

Ryan is more on top of the PR at this point

@ryan-roemer
Copy link
Member

I've added the build task. I think we're otherwise ready for this PR with all the rest of the follow-on work ticketed.

@carloskelly13 carloskelly13 merged commit 337d614 into main Feb 25, 2023
@carloskelly13 carloskelly13 deleted the chore/spectacle-mdx-loader-addition branch February 25, 2023 03:47
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.

Infra: Bring spectacle-mdx-loader in to the monorepo
4 participants