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

Remove references to the <Markdown> component #1045

Merged
merged 8 commits into from
Jul 26, 2022
Merged

Conversation

matthewp
Copy link
Contributor

What kind of changes does this PR include?

  • New or updated content

Description

@netlify
Copy link

netlify bot commented Jul 20, 2022

Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit f8564c1
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/62dfd715dcd6bf000788267f
😎 Deploy Preview https://deploy-preview-1045--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@hippotastic
Copy link
Contributor

hippotastic commented Jul 20, 2022

The more I think about the removal of the section ### <Markdown /> causing hard to resolve link issues in translated versions, the more I like your idea of leaving that section in there for now, @matthewp, but instead of hiding it, replacing the warning content with something along the lines of "This component is no longer built into Astro".

As soon as the translated pages that were previously referring to this component have been updated by the translators, we can then remove the section entirely in a second step (in a separate PR).

@delucis delucis mentioned this pull request Jul 20, 2022
8 tasks
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.

Love this fix and the green CI!

One note: we’ve just added an RC section in the migration guide, so we should probably also include a short note there saying something like

### `<Markdown />` Component Removed

If you were previously using Astro’s built-in `<Markdown />` component, this has been moved to a separate package. You will now need to install `@astrojs/markdown` and update your imports accordingly. For more details, see [the `@astrojs/markdown` README](...readme-link).

@sarah11918
Copy link
Member

@matthewp I just updated the branch , so now this branch should have the latest migration guide with the RC changes section Chris is talking about.

There are already some items in there, so something like @delucis 's suggestion can be added to match the style and then... did we decide this is good to go now?? (LGTM re: proofreading)

@sarah11918 sarah11918 added the improve documentation Enhance existing documentation (e.g. add an example, improve description) label Jul 22, 2022
@matthewp
Copy link
Contributor Author

Updated with the suggested Migration note.

@@ -42,6 +42,10 @@ Astro no longer supports components or JSX expressions in Markdown pages by defa

To make migrating easier, a new [legacy flag](/en/reference/configuration-reference/#legacyastroflavoredmarkdown) can be used to re-enable previous Markdown features.

### `<Markdown />` Component Removed

If you were previously using Astro’s built-in `<Markdown />` component, this has been moved to a separate package. You will now need to install `@astrojs/markdown` and update your imports accordingly. For more details, see [the `@astrojs/markdown` README](https://github.com/withastro/astro/tree/main/packages/markdown/component).
Copy link
Member

Choose a reason for hiding this comment

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

Thanks @matthewp !

@delucis , I'm assuming that this @astrojs/markdown component does NOT get automatically added to our Integrations page?

  • Do we want a legacy package on our integrations page, so that we don't need to send people externally to the README but can instead have a page in Docs? Or, do we specifically NOT want this showing up in Docs, because we don't want people using it unless they really have to?

  • Do we want to take this opportunity here in the migration guide to encourage people to transition to MDX, or simply say, "Here's where the new package is?"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we don't want people using it unless they really have to.

Agree that it's a good opportunity to push people towards MDX, I'll add that.

src/pages/en/migrate.md Outdated Show resolved Hide resolved
src/pages/en/migrate.md Outdated Show resolved Hide resolved
matthewp and others added 2 commits July 26, 2022 07:59
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
@matthewp matthewp merged commit c4bb244 into main Jul 26, 2022
@matthewp matthewp deleted the remove-markdown branch July 26, 2022 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improve documentation Enhance existing documentation (e.g. add an example, improve description)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants