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

Update remark/rehype dependencies #9003

Closed
wants to merge 3 commits into from
Closed

Conversation

mashehu
Copy link

@mashehu mashehu commented Nov 6, 2023

The remark/rehype updates change the minimum required node version to 18. Not sure if that is then also needed for this package.

This resolves the following error for me:

TypeError: Failed to parse Markdown file "/Users/matho180/nf-core.astro/src/content/about/about.md":
Cannot read properties of undefined (reading 'inTable')
    at Object.exitCodeText (file:///Users/matho180/nf-core.astro/node_modules/mdast-util-gfm-table/lib/index.js:123:17)
    at compile (file:///Users/matho180/nf-core.astro/node_modules/mdast-util-from-markdown/lib/index.js:352:40)
    at fromMarkdown (file:///Users/matho180/nf-core.astro/node_modules/mdast-util-from-markdown/lib/index.js:187:29)
    at parser (file:///Users/matho180/nf-core.astro/node_modules/@astrojs/markdown-remark/node_modules/remark-parse/lib/index.js:18:12)
    at Function.parse (file:///Users/matho180/nf-core.astro/node_modules/@astrojs/markdown-remark/node_modules/unified/lib/index.js:273:12)
    at executor (file:///Users/matho180/nf-core.astro/node_modules/@astrojs/markdown-remark/node_modules/unified/lib/index.js:393:31)
    at new Promise (<anonymous>)
    at Function.process (file:///Users/matho180/nf-core.astro/node_modules/@astrojs/markdown-remark/node_modules/unified/lib/index.js:380:14)
    at Object.render (file:///Users/matho180/nf-core.astro/node_modules/@astrojs/markdown-remark/dist/index.js:89:35)
    at Object.load (file:///Users/matho180/nf-core.astro/node_modules/astro/dist/vite-plugin-markdown/index.js:62:46)

Changes

  • What does this change?
  • Be short and concise. Bullet points can help!
  • Before/after screenshots can help as well.
  • Don't forget a changeset! pnpm exec changeset

Testing

No new tests needed. Old tests should catch changes introduced with this PR.

Docs

No new docs needed. It's just an update of the dependencies.

The remark/rehype updates change the minimum required node version to 18. Not sure if that is then also needed for this package. 

This resolves the following error for me:
```
TypeError: Failed to parse Markdown file "/Users/matho180/nf-core.astro/src/content/about/about.md":
Cannot read properties of undefined (reading 'inTable')
    at Object.exitCodeText (file:///Users/matho180/nf-core.astro/node_modules/mdast-util-gfm-table/lib/index.js:123:17)
    at compile (file:///Users/matho180/nf-core.astro/node_modules/mdast-util-from-markdown/lib/index.js:352:40)
    at fromMarkdown (file:///Users/matho180/nf-core.astro/node_modules/mdast-util-from-markdown/lib/index.js:187:29)
    at parser (file:///Users/matho180/nf-core.astro/node_modules/@astrojs/markdown-remark/node_modules/remark-parse/lib/index.js:18:12)
    at Function.parse (file:///Users/matho180/nf-core.astro/node_modules/@astrojs/markdown-remark/node_modules/unified/lib/index.js:273:12)
    at executor (file:///Users/matho180/nf-core.astro/node_modules/@astrojs/markdown-remark/node_modules/unified/lib/index.js:393:31)
    at new Promise (<anonymous>)
    at Function.process (file:///Users/matho180/nf-core.astro/node_modules/@astrojs/markdown-remark/node_modules/unified/lib/index.js:380:14)
    at Object.render (file:///Users/matho180/nf-core.astro/node_modules/@astrojs/markdown-remark/dist/index.js:89:35)
    at Object.load (file:///Users/matho180/nf-core.astro/node_modules/astro/dist/vite-plugin-markdown/index.js:62:46)
```
Copy link

changeset-bot bot commented Nov 6, 2023

🦋 Changeset detected

Latest commit: 1950f96

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 the feat: markdown Related to Markdown (scope) label Nov 6, 2023
@github-actions github-actions bot added the pr: docs A PR that includes documentation for review label Nov 6, 2023
@mashehu
Copy link
Author

mashehu commented Nov 6, 2023

Needs silvenon/remark-smartypants#78 to be merged first, to fix type error.

@bluwy
Copy link
Member

bluwy commented Nov 6, 2023

Aren't these breaking changes? It wouldn't be safe to bump the majors until we release Astro 4. We do have backlogs to upgrade our remark/rehype/mdx deps etc when we start to work on Astro 4 though.

@mashehu
Copy link
Author

mashehu commented Nov 6, 2023

I don't think that these are breaking changes. The only change with the new dependencies is the minimum node version, which astro is already ahead of. Or do you mean because of the underlying changes of remark/rehype plugins?

@ematipico
Copy link
Member

@mashehu it seems the build is failing, please review it

@mashehu
Copy link
Author

mashehu commented Nov 6, 2023

Yes, it needs an update of the types in remark-smartypants (PR with changes is here silvenon/remark-smartypants#78). Should I set this PR as draft until that is fixed? (sorry, I am a first time contributor, so not 100% sure how you handle these things)

@ematipico
Copy link
Member

Yeah it's best to have it as a draft for now. Once it's ready to be reviewed, you can mark it as ready for review. Thank you for understanding!

@mashehu mashehu marked this pull request as draft November 6, 2023 15:58
@bluwy
Copy link
Member

bluwy commented Nov 7, 2023

I don't think that these are breaking changes. The only change with the new dependencies is the minimum node version, which astro is already ahead of. Or do you mean because of the underlying changes of remark/rehype plugins?

We allow users to configure remark/rehype plugins under markdown.remarkPlugins and markdown.rehypePlugins, so they will be affected. Last I checked, the new majors introduce breaking APIs, e.g.

@bluwy
Copy link
Member

bluwy commented Nov 21, 2023

We've upgraded the dependencies in #9138 (for Astro 4). Closing this for now, thanks for the initial PR!

@bluwy bluwy closed this Nov 21, 2023
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) pr: docs A PR that includes documentation for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants