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

fix(deps): update remark monorepo (major) #693

Merged
merged 4 commits into from Oct 17, 2020

Conversation

renovate[bot]
Copy link
Contributor

@renovate renovate bot commented Oct 5, 2020

This PR contains the following updates:

Package Type Update Change
remark-frontmatter dependencies major 2.0.0 -> 3.0.0
remark-react dependencies major 7.0.1 -> 8.0.0

Release Notes

remarkjs/remark-frontmatter

v3.0.0

Compare Source

remarkjs/remark-react

v8.0.0

Compare Source

  • f1c83f9 Update dependencies
    (breaking, potentially, see commit for details)

Renovate configuration

📅 Schedule: "before 3am on Monday" (UTC).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

♻️ Rebasing: Renovate will not automatically rebase this PR, because other commits have been found.

👻 Immortal: This PR will be recreated if closed unmerged. Get config help if that's undesired.


  • If you want to rebase/retry this PR, check this box

This PR has been generated by WhiteSource Renovate. View repository job log here.

@renovate renovate bot added the 🤖 Type: Dependencies Dependency updates or something similar label Oct 5, 2020
@changeset-bot
Copy link

changeset-bot bot commented Oct 5, 2020

🦋 Changeset detected

Latest commit: 2c81526

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

This PR includes changesets to release 7 packages
Name Type
@commercetools-docs/gatsby-theme-docs Patch
@commercetools-docs/gatsby-theme-api-docs Patch
@commercetools-docs/gatsby-theme-code-examples Patch
@commercetools-docs/gatsby-theme-constants Patch
@commercetools-website/api-docs-smoke-test Patch
@commercetools-website/docs-smoke-test Patch
@commercetools-website/site-template Patch

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

@vercel vercel bot temporarily deployed to Preview October 5, 2020 00:13 Inactive
@vercel
Copy link

vercel bot commented Oct 5, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/commercetools/commercetools-docs-kit/fp5utsnk4
✅ Preview: https://commercetools-docs-kit-git-renovate-major-remark-monorepo.commercetools.vercel.app

@emmenko emmenko marked this pull request as draft October 5, 2020 13:22
@emmenko
Copy link
Member

emmenko commented Oct 5, 2020

This requires some investigation around the usage of the commonmark: true option, which has been removed now.

syntax-tree/mdast-util-to-hast@142e973

cc @nkuehn

@renovate renovate bot force-pushed the renovate/major-remark-monorepo branch from 488255c to b8eaeef Compare October 6, 2020 16:13
@vercel vercel bot temporarily deployed to Preview October 6, 2020 16:13 Inactive
@nkuehn
Copy link
Collaborator

nkuehn commented Oct 6, 2020

@emmenko good catch thanks!

remark-react 8 is using an alpha of remark 13, so we'll have to wait here. ( https://github.com/remarkjs/remark-react/blob/8.0.0/package.json#L54 )

remark 13 switches to micromark, which again defaults to commonmark, so we'll maybe have to wait until remark 13 is released and we can upgrade everything remark related at once.
remarkjs/remark#536

@renovate renovate bot force-pushed the renovate/major-remark-monorepo branch from b8eaeef to f80e571 Compare October 6, 2020 16:51
@vercel vercel bot temporarily deployed to Preview October 6, 2020 16:51 Inactive
@renovate renovate bot force-pushed the renovate/major-remark-monorepo branch from f80e571 to 87e7326 Compare October 6, 2020 19:42
@vercel vercel bot temporarily deployed to Preview October 6, 2020 19:42 Inactive
@wooorm
Copy link

wooorm commented Oct 7, 2020

This PR should be good to go if you switch to the alpha of remark-parse, remark-parse": "9.0.0-alpha.1. The remark plugin in that package.json are updated

@emmenko
Copy link
Member

emmenko commented Oct 7, 2020

@wooorm thanks for the hint. We'll try that!

@vercel vercel bot temporarily deployed to Preview October 7, 2020 14:38 Inactive
@emmenko
Copy link
Member

emmenko commented Oct 7, 2020

@wooorm using version 9.0.0-alpha.1 seems to fix most of the issue.
The only think that looks broken now is the markdown table, which don't seem to get parsed.

image

Do we need to change/configure something in that regards? Thanks

@wooorm
Copy link

wooorm commented Oct 7, 2020

Please see the aforementioned issue, it mentions that you now need remark-gfm for gfm

@vercel vercel bot temporarily deployed to Preview October 7, 2020 18:47 Inactive
@emmenko
Copy link
Member

emmenko commented Oct 7, 2020

Thanks again, I missed that point.

@emmenko emmenko marked this pull request as ready for review October 7, 2020 19:03
@emmenko emmenko requested a review from nkuehn October 7, 2020 19:03
@wooorm
Copy link

wooorm commented Oct 7, 2020

Looks wonderful!

@emmenko
Copy link
Member

emmenko commented Oct 12, 2020

@nkuehn should we put this on hold, are you comfortable shipping this? We can also wait that the stable 9.0.0 is released.

@nkuehn
Copy link
Collaborator

nkuehn commented Oct 12, 2020

(lost my whole explanation by closing the browser - so here's a short form only)

TL;DR: We actually don't want tables in this fragment renderer, which is intended for MD snippets rendered inside lists, table cells and other smaller data driven compartments. The whole fuss with overriding the table rendering with a bullet list was just here because the current code seems to have silently do GFM (or parts of it) without having to explicitly ask it. Autolinks are even actively discouraged in documentation, you want choice over whether a string that looks like a URL is a link or not.

  • I vote for removing GFM support here altogether and remove the test case (which was just there to test the workaround). I don't know of production code that has tables in MD fragments or lead paragraph snippets.
  • I would prefer freezing this until remark is released as stable. No urge to use alpha release software without specific need.

@wooorm
Copy link

wooorm commented Oct 12, 2020

to add about remark: a) it’s table, I’ll publish it tomorrow as is, b) gfm is no longer on by default, but in remark-gfm!

Copy link
Collaborator

@nkuehn nkuehn left a comment

Choose a reason for hiding this comment

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

"pre-approve" for when the new remark is released stable. Thanks for your help @wooorm !

@emmenko emmenko force-pushed the renovate/major-remark-monorepo branch from 18e9fed to 2c81526 Compare October 16, 2020 13:14
@vercel vercel bot temporarily deployed to Preview October 16, 2020 13:14 Inactive
@emmenko

This comment has been minimized.

@emmenko emmenko merged commit 94ffef6 into master Oct 17, 2020
@emmenko emmenko deleted the renovate/major-remark-monorepo branch October 17, 2020 09:55
@ghost ghost mentioned this pull request Oct 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 Type: Dependencies Dependency updates or something similar
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants