Markdownz: replace remark with markdownz #5352

eatyourgreens · 2023-09-17T15:59:10Z

Replace remark with markdownz, to parse Zooniverse markdown to HTML, and rehype, to render the parsed HTML as a React component tree.

Bump @zooniverse/react-components to 1.8.0.

Our markdown-it plugins rely on Node modules. I've added fallbacks to the dev classifier and project Webpack builds, where they require them.

~~Rehype 12+ is only available as ESM, so I've installed Rehype v11.~~ Rehype is bundled with markdownz.

~~includes Display Media images inline #5341.~~
closes Migrate Markdownz's editor and help #372.
closes Upgrade Remark #2607 but potentially opens another issue, in that Rehype 12+ is also only available as ESM (Pure ES Modules break the Mocha tests #3779.)
closes Links with +tab+ inconsistent between PFE and FEM markdownz#102.
fixes Markdownz: paragraph text rendered as subscript #5439.
fixes Markdownz: Embedded video overflows its container and doesn't play #5440.

Package

app-content-pages
app-project
app-root
lib-classifier
lib-react-components

How to Review

I've updated the Markdownz Grid Example story to add various examples of Zooniverse-flavoured markdown.
http://localhost:6007/?path=/story/components-markdownz--grid-example

I've also deployed this to kubernetes, so that we can check various production projects:

https://fe-project-branch.preview.zooniverse.org/projects/brooke/i-fancy-cats?env=staging (subject metadata includes URLs.)
https://fe-project-branch.preview.zooniverse.org/projects/zookeeper/galaxy-zoo
https://fe-project-branch.preview.zooniverse.org/projects/zooniverse/gravity-spy
https://fe-project-branch.preview.zooniverse.org/projects/zooniverse/snapshot-serengeti
https://fe-project-branch.preview.zooniverse.org/projects/humphrydavy/davy-notebooks-project/classify/workflow/24833
https://fe-project-branch.preview.zooniverse.org/projects/penguintom79/penguin-watch/about/faq (an example of table-of-contents links, using fragment identifiers.)
https://fe-project-branch.preview.zooniverse.org/projects/sassydumbledore/chimp-and-see/classify/workflow/18435 (embedded YouTube videos in the field guide.)
https://fe-project-branch.preview.zooniverse.org/projects/sassydumbledore/chimp-and-see/about/research (markdown tables used to layout images next to text.)
https://fe-project-branch.preview.zooniverse.org/projects/rsengar/pulsar-seekers/about/research (embedded YouTube video about the project.)

Checklist

PR Creator - Please cater the checklist to fit the review needed for your code changes.
PR Reviewer - Use the checklist during your review. Each point should be checkmarked or discussed before PR approval.

General

Tests are passing locally and on Github
Documentation is up to date and changelog has been updated if appropriate
You can yarn panic && yarn bootstrap or docker-compose up --build and FEM works as expected
FEM works in all major desktop browsers: Firefox, Chrome, Edge, Safari (Use Browserstack account as needed)
FEM works in a mobile browser

General UX

Example Staging Project: i-fancy-cats

All pages of a FEM project load: Home Page, Classify Page, and About Pages
Can submit a classification
Can sign-in and sign-out
The component is accessible
- Can be used with a screen reader BBC guide to VoiceOver
- Can be used from the keyboard WebAIM guide to keyboard testing
- It is passing accessibility checks in the storybook

Refactoring

The PR creator has described the reason for refactoring
The refactored component(s) continue to work as expected

eatyourgreens · 2023-09-17T16:45:22Z

packages/lib-react-components/src/Markdownz/Markdownz.spec.js

-    expect(container.querySelectorAll('a[href]')).to.have.lengthOf(12)
-    expect(container.querySelectorAll('p')).to.have.lengthOf(17)
+    expect(container.querySelectorAll('a[href]')).to.have.lengthOf(10)
+    expect(container.querySelectorAll('p')).to.have.lengthOf(18)


These changes get the tests passing, but I can't figure out, from the example markdown, what the expected values really should be.

coveralls · 2023-09-17T16:46:42Z

coverage: 82.481% (-0.04%) from 82.524% when pulling a0b2cbe on rehype-markdownz into af85940 on master.

eatyourgreens · 2023-09-18T07:17:13Z

Had to install Rehype 11, because Rehype 12+ is only available as ESM.

eatyourgreens · 2023-09-18T09:16:00Z

Rehype parses a HTML fragment as a full page, wrapped in <html>, which results in an invalid DOM. See the Rehype examples.

<html> nested inside <div> on the Gravity Spy Research page.

Fixed by explicitly setting fragment: true.

eatyourgreens · 2023-09-18T21:51:39Z

markdown-it was last updated a year ago. That doesn’t look good.
markdown-it/markdown-it#948

goplayoutside3 · 2023-09-20T02:40:49Z

Re: Rehype 12+ is only available as ES -

I didn't realize that we'd run into the same headache of ESM-only package by switching to rehype so thank you for noting that right away! I think I'm hesitant about replacing remark with rehype because it seems like a temporary solution and also hesitant that markdown-it hasn't been updated recently (as you noted). What are your thoughts on FEM markdown strategy going forward? What's your opinion on switching to markdownz to address the Github Issues linked, versus sticking it out with Remark for now?

eatyourgreens · 2023-09-20T10:03:55Z

What's your opinion on switching to markdownz to address the Github Issues linked, versus sticking it out with Remark for now?

Depends on what stakeholders, which would be project teams here, expect from Zooniverse-flavoured markdown. If they expect their markdown to continue to work without changes, then markdownz will continue to parse it to the same HTML that they're already accustomed to.

With Remark, the input markdown changes and the output HTML is different, which should be summarised in ADR 11. So project teams will have to edit their existing markdown for the new parser, and we will need to document the required changes for them. We also need to install or write Remark plugins to handle custom features of Zooniverse-flavoured markdown, including:

username mentions.
subject mentions.
hashtag links.
+tab+ link syntax for new tabs.
[[toc]] for tables of contents.
custom image sizing.
embedded HTML5 audio and video.

My personal preference would be to leave Zooniverse-flavoured markdown unchanged if possible, so use markdownz to convert markdown to HTML.

NB. Remark uses Rehype to convert its output HTML to a React component tree, and remark-react has been replaced by rehype-react. Whether we use Remark or markdownz to parse markdown to HTML, we'll still need Rehype to map the output HTML to Zooniverse Grommet components.

Re. ESM: that's a wider issue that affects all our dependencies. This monorepo was built on CommonJS, at a time when CommonJS was being replaced by ES modules but import/export wasn't supported in Node. For example, several of the packages in the monorepo are written as CommonJS modules. These packages are all written with require and module.exports:

lib-async-states
lib-grommet-theme
lib-panoptes-js

The config files and custom Express servers for both NextJS apps are also written in CommonJS.

We need to be able to build and test in a world where not all of our dependencies are CommonJS any more. Whichever markdown parser we choose to use, it's not likely to be available as a CommonJS module forever. I've made markdownz available as both CommonJS and ESM since markdownz version 8. Unified packages, including Rehype and Remark, are only published as ESM.

Bun has a good summary of the current state of Node module resolution.

eatyourgreens · 2023-10-31T11:58:00Z

The new pulsars project uses a YouTube video:
https://master.pfe-preview.zooniverse.org/projects/rsengar/pulsar-seekers/about/research

I've added it to the list of examples at the top of this PR.

goplayoutside3 · 2023-11-01T00:37:29Z

The links to example projects are helpful - thank you! markdownz updates are looking good, and I think this PR is good to go 👍 👍 👍

Because of the change to external links (requiring +tab+ to open new browser tab), I propose this gets merged directly AFTER this week's FEM deploy. There are definitely changes to markdown behavior here that live FEM project teams should be aware of. For example, there are links in the Davy Notebooks banner (this branch, production) and the Davy Notebooks team + volunteers are accustomed to those opening in a new tab, especially from the classify page.

Let's notify the #project-building channel after this gets merged tomorrow about changes to markdown behavior going live on next FEM deploy (November 7). That'll give PMs a week or so to communicate to project teams.

Replace `remark` with `markdownz`, to parse Zooniverse markdown to HTML, and `rehype`, to render the parsed HTML as a React component tree. Bump `@zooniverse/react-components` to 1.8.0. Our `markdown-it` plugins rely on Node modules. I've added fallbacks to the dev classifier and project Webpack builds, where they require them.

8.5.0 introduces a `useMarkdownz` hook.

`markdownz` 9.0.0 includes `rehype`, so we can remove it here.

Inject parsed markdown with the `useMarkdownz` hook.

eatyourgreens added the refactor Refactoring existing code label Sep 17, 2023

eatyourgreens requested a review from goplayoutside3 September 17, 2023 15:59

eatyourgreens force-pushed the rehype-markdownz branch from 30de769 to fb83f3c Compare September 17, 2023 16:40

eatyourgreens commented Sep 17, 2023

View reviewed changes

eatyourgreens force-pushed the rehype-markdownz branch 2 times, most recently from dad57d4 to 9f05e22 Compare September 17, 2023 17:07

goplayoutside3 self-assigned this Sep 18, 2023

eatyourgreens added the ESM label Sep 18, 2023

eatyourgreens force-pushed the rehype-markdownz branch from 9f05e22 to 89905b6 Compare September 18, 2023 07:12

eatyourgreens force-pushed the rehype-markdownz branch 2 times, most recently from aa48a48 to db5b846 Compare September 18, 2023 08:42

eatyourgreens requested a review from lcjohnso September 18, 2023 09:05

eatyourgreens marked this pull request as ready for review September 18, 2023 09:06

eatyourgreens force-pushed the rehype-markdownz branch 4 times, most recently from 1fabec3 to 412cf85 Compare September 18, 2023 17:17

eatyourgreens mentioned this pull request Sep 18, 2023

Migrate Markdownz's editor and help #372

Closed

1 task

eatyourgreens force-pushed the rehype-markdownz branch 4 times, most recently from 50dd682 to 38f033f Compare September 19, 2023 16:26

eatyourgreens force-pushed the rehype-markdownz branch from 38f033f to c5f9f8b Compare September 20, 2023 09:41

eatyourgreens force-pushed the rehype-markdownz branch from c5f9f8b to 4ec8ccf Compare September 20, 2023 10:17

eatyourgreens force-pushed the rehype-markdownz branch from f740e04 to 4a4b82b Compare October 31, 2023 22:00

goplayoutside3 approved these changes Nov 1, 2023

View reviewed changes

github-actions bot added the approved This PR is approved for merging label Nov 1, 2023

eatyourgreens enabled auto-merge (squash) November 1, 2023 16:55

eatyourgreens added 21 commits November 1, 2023 16:55

Update the markdown grid story

139afea

Replace twemoji images with their alt text

de68937

Bump markdownz to 8.1.2

4968b9d

Preserve heading IDs for fragment links

2e6203d

Refactor storybook examples

5c1a2e2

Bump markdownz to 8.1.4

c5c6d1c

Style Markdownz output components

208328f

Bump markdownz to 8.2.0

cfee4bd

Bump markdownz to 8.3.2

0cc3bf7

Use a three-column table in the storybook

e47f5f5

Replace 'forceInline' with 'inline'

d047583

Bump Markdownz to 8.3.3

25f2d4b

Bump markdownz to 8.4.1

90b39fe

Move 'table' to the top of the examples

98a5a07

Bump markdownz to 8.5.0

b841c39

8.5.0 introduces a `useMarkdownz` hook.

Add an ADR

f61da62

Bump markdownz to 9.0.0

9f610d8

`markdownz` 9.0.0 includes `rehype`, so we can remove it here.

Bump markdownz to 9.1.0

66b5a34

Inject parsed markdown with the `useMarkdownz` hook.

Remove unist-util-visit

959b554

Bump markdownz to 9.1.3

a0b2cbe

eatyourgreens force-pushed the rehype-markdownz branch from 4a4b82b to a0b2cbe Compare November 1, 2023 16:55

eatyourgreens merged commit b02ac8d into master Nov 1, 2023
7 checks passed

eatyourgreens deleted the rehype-markdownz branch November 1, 2023 17:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Markdownz: replace remark with markdownz #5352

Markdownz: replace remark with markdownz #5352

eatyourgreens commented Sep 17, 2023 •

edited

Loading

eatyourgreens Sep 17, 2023

coveralls commented Sep 17, 2023 •

edited

Loading

eatyourgreens commented Sep 18, 2023

eatyourgreens commented Sep 18, 2023 •

edited

Loading

eatyourgreens commented Sep 18, 2023

goplayoutside3 commented Sep 20, 2023

eatyourgreens commented Sep 20, 2023 •

edited

Loading

eatyourgreens commented Oct 31, 2023 •

edited

Loading

goplayoutside3 commented Nov 1, 2023

Markdownz: replace remark with markdownz #5352

Markdownz: replace remark with markdownz #5352

Conversation

eatyourgreens commented Sep 17, 2023 • edited Loading

Package

How to Review

Checklist

General

General UX

Refactoring

eatyourgreens Sep 17, 2023

Choose a reason for hiding this comment

coveralls commented Sep 17, 2023 • edited Loading

eatyourgreens commented Sep 18, 2023

eatyourgreens commented Sep 18, 2023 • edited Loading

eatyourgreens commented Sep 18, 2023

goplayoutside3 commented Sep 20, 2023

eatyourgreens commented Sep 20, 2023 • edited Loading

eatyourgreens commented Oct 31, 2023 • edited Loading

goplayoutside3 commented Nov 1, 2023

eatyourgreens commented Sep 17, 2023 •

edited

Loading

coveralls commented Sep 17, 2023 •

edited

Loading

eatyourgreens commented Sep 18, 2023 •

edited

Loading

eatyourgreens commented Sep 20, 2023 •

edited

Loading

eatyourgreens commented Oct 31, 2023 •

edited

Loading