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

Markdown and MDX configuration rework #5684

Merged
merged 29 commits into from
Jan 3, 2023

Conversation

bholmesdev
Copy link
Contributor

@bholmesdev bholmesdev commented Dec 27, 2022

Changes

Refine Markdown and MDX configuration options for ease-of-use. Decisions based on a discord feedback thread.

Markdown

  • Remove remark-smartypants from Astro's default Markdown plugins.
  • Replace the extendDefaultPlugins option with a simplified gfm boolean. This is enabled by default, and can be disabled to remove GitHub-Flavored Markdown.
  • Ensure GitHub-Flavored Markdown is applied whether or not custom remarkPlugins or rehypePlugins are configured. If you want to apply custom plugins and remove GFM, manually set gfm: false in your config.

MDX

  • Support all Markdown configuration options (except drafts) from your MDX integration config. This includes syntaxHighlighting and shikiConfig options to further customize the MDX renderer.
  • Simplify extendDefaults to an extendMarkdownConfig option. MDX options will be deeply merged with your Markdown config by default. By setting extendMarkdownConfig to false, you can "eject" to set your own syntax highlighting and plugins.

Testing

  • Update markdown-plugins and mdx-plugins test for new config extend options
  • Add MDX test for syntax highlight inheritance

Docs

@bholmesdev bholmesdev requested a review from a team as a code owner December 27, 2022 23:29
@changeset-bot
Copy link

changeset-bot bot commented Dec 27, 2022

🦋 Changeset detected

Latest commit: aae0f82

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope) feat: markdown Related to Markdown (scope) labels Dec 27, 2022
import { unified } from 'unified';
import { VFile } from 'vfile';

export { rehypeHeadingIds } from './rehype-collect-headings.js';
export * from './types.js';

export const DEFAULT_REMARK_PLUGINS = ['remark-gfm', 'remark-smartypants'];
export const DEFAULT_REHYPE_PLUGINS = [];
export const markdownConfigDefaults: Omit<Required<AstroMarkdownOptions>, 'drafts'> = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move markdownConfigDefaults to a const so Remark, Astro Core, and MDX can all share the same fallbacks!

@lorenzolewis
Copy link
Contributor

I just put in this RFC discussion, I wonder if this might complement that withastro/roadmap#424

@bholmesdev
Copy link
Contributor Author

@lorenzolewis thanks! That's a thorough set of ideas I think should be explored in future RFCs. Let me know if any changes proposed here would be blocking to your proposals. Otherwise, we'll treat this PR as step 1 on that journey!

Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

I love this new config, @bholmesdev! Much more intuitive! 🥳

Some comments below!

packages/astro/src/@types/astro.ts Outdated Show resolved Hide resolved
packages/integrations/mdx/README.md Outdated Show resolved Hide resolved
packages/integrations/mdx/README.md Outdated Show resolved Hide resolved
packages/integrations/mdx/README.md Outdated Show resolved Hide resolved
packages/integrations/mdx/README.md Outdated Show resolved Hide resolved

You can set remark-rehype options in your config file:
For example, say you need to disable GitHub-Flavored Markdown and apply an extra remark plugin for MDX files. You can apply these options like so, with `extendMarkdownConfig` enabled by default:
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering whether these examples might be better suited to the docs page itself? I don't think these tell me anything new. They visually show what it looks like when both Markdown and MDX have config options set, but I'm not sure that's needed here?

The docs page here will need updating anyway, to correspond to these changes: /#configuring-markdown-and-mdx

Maybe what you have here as examples actually replaces what's there? Since that's where we show what it looks like when both have config settings are applied.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed! I'll put a docs PR on-hold until we get a dev approval here. May be some final API reshuffling.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I think we have dev consensus. Moving to a docs PR!

- [`recmaPlugins`](#recmaplugins)

### `extendPlugins`
### Markdown configuration options
Copy link
Member

Choose a reason for hiding this comment

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

nit: it feels weird to call these "Markdown configuration options"

I know this is shorthand for mirrors "all the stuff you can configure in Markdown, too!" but it really is MDX configuration options, that happen to (except for one 😅) be the exact same options that are available elsewhere for configuring Markdown.

I might be tempted to still, in fact, call this MDX configuration options. I think that's what it is. And, I think if people are searching for "MDX configuration" then having that in a header is helpful for our Algolia search results.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, the overarching section is already MDX configuration, so I don't think that would make sense. I vote removing the heading if that's a concern 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update: renamed to "Options inherited from Markdown config" to clarify. If we don't like that heading, I say scrap it. I was just struggling with the table-of-contents feeling awkward without a heading present here.

Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

In general, I think this direction is great! Won’t dive into the inline docs before we know we’re doing this, but left one comment that might help us think through these changes from the perspective of an (existing) user.

.changeset/shaggy-keys-turn.md Outdated Show resolved Hide resolved
@lorenzolewis
Copy link
Contributor

@bholmesdev I put some details on a possible enum value in the flag that could replace the markdown.githubFlavoredMarkdown config value in withastro/roadmap#424 (comment).

It might allow for more flexibility in the future without a major semver change, but 💯 up to your discretion if that's something you feel is relevant at this stage or not.

@bholmesdev
Copy link
Contributor Author

@lorenzolewis thanks! I just left a comment on that enum suggestion.

To explain in context: I'm not sure I'm comfortable with blanket terms like basic or enhanced when we're unsure what use cases these will cover. I also have a feeling properties like githubFlavoredMarkdown will stick around even with this enum approach for finer tuning. For simplicity, I'd prefer to keep this PR as-is for the next major release. I think we can experiment with further Markdown features as a separate integration and even an experimental flag in Astro core for version 2.X.

@bholmesdev bholmesdev added the semver: major Change triggers a `major` release label Dec 30, 2022
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

Looks good to me! Didn't see anything that stands out odd, and the new defaults looks great.

@bholmesdev bholmesdev force-pushed the feat/markdown-plugin-config-rework branch from 1944ffb to 2d5a774 Compare January 2, 2023 15:46
```


Additionally, applying remark and rehype plugins **no longer disables** `githubFlavoredMarkdown`. You will need to opt-out manually by setting `githubFlavoredMarkdown` to `false`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I really like this part

@matthewp matthewp added needs discussion Issue needs to be discussed and removed needs discussion Issue needs to be discussed labels Jan 3, 2023
Copy link
Contributor

@matthewp matthewp left a comment

Choose a reason for hiding this comment

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

Needs TSC approval.

@matthewp matthewp added needs discussion Issue needs to be discussed and removed needs discussion Issue needs to be discussed labels Jan 3, 2023
Copy link
Contributor

@matthewp matthewp left a comment

Choose a reason for hiding this comment

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

Approved by TSC.

@bholmesdev
Copy link
Contributor Author

Approved by TSC.

Thanks TypeScript Compiler ❤️

@bholmesdev bholmesdev force-pushed the feat/markdown-plugin-config-rework branch from 1514417 to aae0f82 Compare January 3, 2023 21:40
@matthewp matthewp merged commit a9c2920 into main Jan 3, 2023
@matthewp matthewp deleted the feat/markdown-plugin-config-rework branch January 3, 2023 22:12
@bholmesdev bholmesdev mentioned this pull request Jan 5, 2023
@sarah11918 sarah11918 mentioned this pull request Jan 24, 2023
23 tasks
delucis added a commit that referenced this pull request Jan 24, 2023
matthewp added a commit that referenced this pull request Jan 24, 2023
* [ci] release

* Wrap astro 2.0 beta logs in `<details>`

* Add link to docs upgrade guide

* First pass cleaning up 2.0 release notes

* mdx changes from Sarah

* combine 5584 and 5842 in deno, image, netlify

* markdown/remark incl (5684 & 5769) to match mdx

* Tweak markdown/remark formatting

* Format astro-prism

* Format astro-rss

* Format create-astro

* Format cloudflare

* Format lit

* Format partytown

* Format node

* Format preact

* Format react

* Format solid

* Format svelte

* Format tailwind

* Format vercel

* Format vue

* Format telemetry

* Format webapi

* Format scripts

* Reinstate h3s for headings

Co-authored-by: Ben Holmes <[email protected]>

* Reformat mdx

* astro & markdown/remark: Combine #5679 & #5684 changelogs

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Chris Swithinbank <[email protected]>
Co-authored-by: Sarah Rainsberger <[email protected]>
Co-authored-by: Ben Holmes <[email protected]>
bholmesdev added a commit that referenced this pull request Jan 24, 2023
* [ci] release

* Update changelogs (#5955)

* [ci] release

* Wrap astro 2.0 beta logs in `<details>`

* Add link to docs upgrade guide

* First pass cleaning up 2.0 release notes

* mdx changes from Sarah

* combine 5584 and 5842 in deno, image, netlify

* markdown/remark incl (5684 & 5769) to match mdx

* Tweak markdown/remark formatting

* Format astro-prism

* Format astro-rss

* Format create-astro

* Format cloudflare

* Format lit

* Format partytown

* Format node

* Format preact

* Format react

* Format solid

* Format svelte

* Format tailwind

* Format vercel

* Format vue

* Format telemetry

* Format webapi

* Format scripts

* Reinstate h3s for headings

Co-authored-by: Ben Holmes <[email protected]>

* Reformat mdx

* astro & markdown/remark: Combine #5679 & #5684 changelogs

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Chris Swithinbank <[email protected]>
Co-authored-by: Sarah Rainsberger <[email protected]>
Co-authored-by: Ben Holmes <[email protected]>

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Matthew Phillips <[email protected]>
Co-authored-by: Chris Swithinbank <[email protected]>
Co-authored-by: Sarah Rainsberger <[email protected]>
Co-authored-by: Ben Holmes <[email protected]>
matthewp added a commit that referenced this pull request Jan 26, 2023
* [ci] release

* Update changelogs (#5955)

* [ci] release

* Wrap astro 2.0 beta logs in `<details>`

* Add link to docs upgrade guide

* First pass cleaning up 2.0 release notes

* mdx changes from Sarah

* combine 5584 and 5842 in deno, image, netlify

* markdown/remark incl (5684 & 5769) to match mdx

* Tweak markdown/remark formatting

* Format astro-prism

* Format astro-rss

* Format create-astro

* Format cloudflare

* Format lit

* Format partytown

* Format node

* Format preact

* Format react

* Format solid

* Format svelte

* Format tailwind

* Format vercel

* Format vue

* Format telemetry

* Format webapi

* Format scripts

* Reinstate h3s for headings

Co-authored-by: Ben Holmes <[email protected]>

* Reformat mdx

* astro & markdown/remark: Combine #5679 & #5684 changelogs

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Chris Swithinbank <[email protected]>
Co-authored-by: Sarah Rainsberger <[email protected]>
Co-authored-by: Ben Holmes <[email protected]>

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Matthew Phillips <[email protected]>
Co-authored-by: Chris Swithinbank <[email protected]>
Co-authored-by: Sarah Rainsberger <[email protected]>
Co-authored-by: Ben Holmes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: markdown Related to Markdown (scope) pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope) semver: major Change triggers a `major` release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants