client-side narrative parsing #1172

jameshadfield · 2020-06-18T04:15:52Z

The narrative markdown file has historically been transformed into JSON format on the server. This is unnecessary and has multiple disadvantages. This PR changes the (default) behavior of auspice so that narrative markdown content is parsed client-side.

The following situations have been tested (here "old" means current master, "new" means this PR)

Old server (API handler) + new client: Narratives are sent in JSON format, the client handles this without problem but displays the following error in the browser console: "Narrative file doesn't appear to be markdown! Attempting to parse as JSON."
New server (API handler) + new client: The markdown file is sent without transformation and parsed client-side.
New server (API handler) + old client: The markdown file will be sent as transformed JSON and will render without issue.
New server (API handler) + old (unchanged) request from a non-auspice client: no change in behavior.

The client-side and server-side parsing of the narrative file is done using the same code with one exception: while they both use the marked library to convert markdown to HTML, they each use different (sanitizer) settings. The server-side parsing uses the unchanged settings from before, however on the client we use the existing parseMarkdown function. For example, the author affiliations (deriving from YAML frontmatter and appearing on the opening slide of a narrative) use the following "markdown":

### Author: Author Name <sup> 1 </sup>
<sub><sup> 1 </sup> string of affiliations</sub>

However the content inside <sub><sup> tags is being removed when parsed on the client. There may be other situations (e.g. embedded tweets).

Testing

Unit tests added to test various possible formats of the YAML frontmatter.

Advantages of client-side parsing

Error messages can be shown in the browser console, giving users more indication of what's gone wrong if things didn't work
Makes live-editing of narrative files easier (see Live editing of narrative files #1117)
Less server overhead
Simplifies custom server creation (no need to transform the file, just return it)

In addition the parsing of narrative files has been made more robust in this PR (this applies to both server-side and client-side parsing as we use the same code).

For more information please see the commit messages.

Outstanding tasks:

Test using nextstrain.org
Adjust sanitizer settings or change how we represent certain fields in the frontmatter
More testing, e.g. rebase onto branch use-narratives-to-test and run integration tests
update "XXX" fields in the docs with up-to-date data
examine changes in bundle structure

… file This is in preparation for client-side parsing of narrative files. A optional `type` URL query allows the same API to return the markdown file (see the documentation changes in this commit for more details). There are other possibilities, such as using the content-type field in the header, but I felt this was the clearest.

Here we add the framework to (eventually) parse the response from `getNarrative` as a markdown file and interpret this client-side. To facilitate backward compatibility in the cases where a server does not respect the `?type=md` query and continues to return a JSON, if the file doesn't look like markdown we will attempt to parse it as JSON. To mimic such a server, we explicitly request "type=json" in this commit, as the code to actually do the client-side md-parsing is not yet implemented.

This commit shifts the parsing of the markdown file (md + yaml frontmatter) into a centralised function which is imported by both the server and the client, allowing us to maintain backward compatible server-side parsing as well as client-side parsing, whilst avoiding code duplication and the inevitable divergence of functionality. Our approach to conversion of YAML frontmatter -> markdown is essentially unchanged. The algorithm for parsing of the main body has been rewritten to be more robust. Unit tests are added to cover the various ways we can define authors, links & affiliations in the YAML frontmatter of narratives.

Parsing of the narrative (YAML) frontmatter produces markdown with `<sup>` and `<sub>` HTML elements to render affiliations using superscripts. (This behavior is unchanged from parsing server-side.) The client-side markdown sanitizer settings, however, were stricter than those employed on the server, and were removing these HTML elements causing the affiliations not to be displayed. These settings are relaxed in this commit.

The move to client-parsing of narratives has caused two changes to bundle structure. Firstly, since the `loadJSON` function now references the markdown parsing code, the dependency graph tries to pull in the related libraries into the chunk with most of the Auspice code (currently chunk 5). Previously these were a separate chunk lazily loaded with (e.g.) the footer parsing component. Since it's very common to display markdown footers, it makes life simple to include these libraries in the other-vendors bundle. It is possible to lazily load the markdown parsing function as needed, but more complicated. Secondly, the YAML frontmatter parser (previously server-side only) has a dependency on `js-yaml` which itself has dependency on "esprima" and others. Esprima is unnecessary for our purposes and we use a webpack loader to ignore it. For simplicity, we shift `js-yaml` to the other-vendors bundle. An alternative approach would be to bundle pre-minified 'js-yaml' bundle.

For unknown reasons, the core-vendors and other-vendors chunks have different hashes when building locally compared with via GitHub CI. This is after running `npm ci` to mimic the CI conditions. An issue should be created to better understand this, as it implies that the contents differ.

Auspice PR nextstrain/auspice#1172 allows narrative parsing to be performed client-side. Once that PR is merged, auspice will make `getNarrative` GET requests with a `type=md` query and expect the markdown content to be returned as the response (although it will still handle a JSON response for backwards compatability.) This PR updates the nextstrain.org server's `/charon/getNarrative` handler to stream the markdown file as a response.

eharkins

Nice work @jameshadfield! Parsing does indeed look more robust (and in practice, handles a recent issue with narrative parsing gracefully) and more importantly, we can feel more confident knowing that any parsing issues will manifest in client errors - making them transparent for the user and less impactful to nextstrain.org as you have pointed out.

As we discussed offline, after rebasing on master, unit tests pass and any failures of integration tests seem like they are not introduced by this PR.

eharkins · 2020-07-23T21:39:55Z

src/util/parseNarrative.js

+  const mainMarkdownLines = [];
+  let inMainMarkdownSection = false;
+  slide.lines.forEach((line) => {
+    if (!inMainMarkdownSection && line.match(/^```auspiceMainDisplayMarkdown\s*$/)) {


Do we want to allow for chars (whitespace or otherwise) on the line before ```auspiceMainDisplayMarkdown and/or non-whitespace chars on that same line after that? I.e. is it important to strictly specify the opening and closing tags for auspiceMainDisplayMarkdown on their own lines without any other characters on those lines? If the answer is yes, that seems perfectly reasonable to me, just checking this is the intention while I'm scrutinizing auspiceMainDisplayMarkdown parsing after recently working on it.

I believe we want to require the line to not have leading whitespace, following markdown syntax for fenced code blocks.

Tangentially, when I implemented this ability it was in a needs-must situation and I didn't foresee how popular it'd be. I can imagine us deprecating the fenced-code-block syntax for something else in the future, e.g. ## Main Screen Markdown.

Auspice PR nextstrain/auspice#1172 allows narrative parsing to be performed client-side. Once that PR is merged, auspice will make `getNarrative` GET requests with a `type=md` query and expect the markdown content to be returned as the response (although it will still handle a JSON response for backwards compatability.) This PR updates the nextstrain.org server's `/charon/getNarrative` handler to stream the markdown file as a response.

jameshadfield temporarily deployed to auspice-parse-narrative-rbinda June 18, 2020 04:16 Inactive

jameshadfield added 2 commits July 13, 2020 16:24

jameshadfield force-pushed the parse-narrative-in-client branch from ce70b81 to b61af18 Compare July 13, 2020 05:27

jameshadfield added 3 commits July 14, 2020 14:41

[client narratives] update documentation

7e2b9d7

jameshadfield force-pushed the parse-narrative-in-client branch from b61af18 to a0cf89e Compare July 14, 2020 02:42

jameshadfield mentioned this pull request Jul 14, 2020

wip: parse narratives on client nextstrain/nextstrain.org#203

Merged

jameshadfield added 2 commits July 15, 2020 14:20

jameshadfield force-pushed the parse-narrative-in-client branch from e039d39 to de9439f Compare July 15, 2020 02:35

kairstenfay self-requested a review July 20, 2020 23:04

jameshadfield mentioned this pull request Jul 21, 2020

rewrite parsing of auspiceMainDisplayMarkdown; #1189

Merged

eharkins approved these changes Jul 23, 2020

View reviewed changes

This was referenced Jul 28, 2020

Improve redux state recalc on narrative slide changes. Closes #1191 #1192

Merged

next release #1193

Merged

jameshadfield merged commit ba652b8 into master Aug 3, 2020

jameshadfield deleted the parse-narrative-in-client branch August 3, 2020 02:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client-side narrative parsing #1172

client-side narrative parsing #1172

jameshadfield commented Jun 18, 2020 •

edited

Loading

eharkins left a comment

eharkins Jul 23, 2020

jameshadfield Jul 23, 2020

client-side narrative parsing #1172

client-side narrative parsing #1172

Conversation

jameshadfield commented Jun 18, 2020 • edited Loading

Testing

Advantages of client-side parsing

eharkins left a comment

Choose a reason for hiding this comment

eharkins Jul 23, 2020

Choose a reason for hiding this comment

jameshadfield Jul 23, 2020

Choose a reason for hiding this comment

jameshadfield commented Jun 18, 2020 •

edited

Loading