Build time tooltips #1334

rogermparent · 2020-05-22T03:03:02Z

This PR loads Tooltip data into GraphQL at build time, processing the Markdown descriptions into HTML. The components that consumed this data now no longer depend on react-markdown, and instead load the build-time HTML content via dangerouslySetInnerHTML.

This lets it be used at build time without transpilation. We could also use JSON, but this change is smaller and easier.

This is similar to the recent Community refactor. A model parses the raw JS glossary data, walks the tree parsing all Markdown to HTML, then exposes the resulting processed glossary via GraphQL and a useStaticQuery hook. The `Tooltip` components that used to use `react-markdown` now use `dangerouslySetInnerHTML` to consume description data.

shcheklein · 2020-05-22T18:40:16Z

A small bug:

Remark dislikes special elements broken across lines halfway, so this is needed to get the link processsed.

rogermparent · 2020-05-22T18:57:18Z

Thanks for finding that!
It was an indentation issue in the glossary source. Remark dislikes having things like links broken midway through and skips over offenders. Similar issues can happen with Rehype and rehype-react components.

calibre-analytics · 2020-05-22T18:58:28Z

You are out of tests

Choose a plan to resume monitoring your Sites and Pull Requests. If you need help, check the Manage Your Plan and Test Usage guide.

shcheklein · 2020-05-22T19:17:08Z

@rogermparent still not fixed for me, deployment problem?

rogermparent · 2020-05-22T19:29:34Z

@rogermparent still not fixed for me, deployment problem?

It's fixed on my local machine, so it looks a lot like a cache issue. Maybe it's part of my contentDigest logic.

YAML is much friendlier than JSON for embedded multiline string content, and with a slight modification to the JSON file loader we can handle them the exact same way and parse the YAML to JSON at build time to serve to the components. With a regular data file instead of a JS module, HMR works for tooltips now (mostly)! We used to have to restart the dev server to update tooltips, and the previous iteration of this branch also had caching issues. This solves that by binding the Glossary data to a file, piggybacking on its `contentDigest`. You still have to refresh the page to update tooltips though, but I think this is more due to how they are implemented with state on the client side. Either way, it's an improvement from before and eliminating the need to restart gets us most of the possible time savings in this regard. - Added YAML support to `json-files` model. I didn't rename it because I think `json-files` more accurately describes how files are parsed into plain JSON than something like `data-files` - Updated Glossary model to use YAML source file - Converted ES5 glossary to YAML - Explicitly add `js-yaml` and run `yarn update` `js-yaml` is used within Gatsby, so it's already installed. However, we get two different versions without an `update`

…ild-time-tooltips

Remark doesn't like breaking specifically between `]` and `(` in a link, but this should be fine.

rogermparent · 2020-05-22T21:50:43Z

I changed the Glossary data file to YAML, as it's the data format most friendly to line breaks (as opposed to JSON). All parsing is done at build time, and components still receive the exact same JSON object as before.

Doing this instead of using an ES5/6 module also allows for HMR on tooltips. The rest of the implementation doesn't quite allow for it yet, but this change allows us to simply refresh the page to update instead of rebuilding the application.

shcheklein · 2020-05-22T22:16:42Z

@rogermparent good stuff! a few questions:

do we run prettier for yml/yaml?
do we use a single Node right now for all glossary entries? would it make sense to actually split the whole file into multiple? we can then probably use regular MD with frontmatter?

rogermparent · 2020-05-22T22:28:26Z

1. do we run prettier for yml/yaml?

I don't think so- at least our format-all command doesn't include it. I can fix that.

2. do we use a single Node right now for all glossary entries? would it make sense to actually split the whole file into multiple? we can then probably use regular MD with frontmatter?

My custom model does that- it mimics the exact data shape we used previously. I can change it to be any data shape we'd like. Our Tooltip component is currently built with a custom state that expects to consume the all the tooltip data as a JSON blob ahead of time and show tooltips when needed. If we split the data into multiple nodes we'll either have to recombine it for the components or change the components to accept piecemeal data.

At this point, the way we store Glossary entries is 100% writer's preference- we can take anything and transform it into the JSON that the frontend components expect during build time for no runtime penalty.

For example, we could have a separate glossary directory in content that is used to hold glossary entries, and then use those entries to make the Tooltip JSON at build time. We can then also consume that data elsewhere.

shcheklein · 2020-05-22T22:46:06Z

For example, we could have a separate glossary directory in content that is used to hold glossary entries, and then use those entries to make the Tooltip JSON at build time. We can then also consume that data elsewhere.

it sound reasonable to me. It's easier for writers, we don't need to add prettier, we don't need converters. WDYT?

rogermparent · 2020-05-22T23:04:03Z

it sound reasonable to me. It's easier for writers, we don't need to add prettier, we don't need converters. WDYT?

Sounds good to me! I'll adapt this branch to that data shape once it deploys with the current one. I had to sort out some merge issues with master.

rogermparent · 2020-05-23T01:12:29Z

I got a good deploy with the original setup on Heroku manually a while ago, though it doesn't show here.
I've got the Glossary working with individual Markdown files now! There's a few decisions we can make here on how we want to handle things:

Primarily, we need to figure out what directory to use for glossary entries and where we want the glossary entries to show up (glossary page, tooltips, and optionally individual pages for entries). I'll explain a few common routes, but we could really do anything here so feel free to make a suggestion for something that I don't describe below.

content/docs/glossary

It's right next to the rest of the docs. Entries will be picked up by the DocsPage Model by default, creating pages at docs/glossary/${term}. The pages work, and are regular doc MD pages that don't show up in the sidebar since the sidebar is manually defined.

To keep glossary entries co-located with docs here, we must either build on these glossary entry pages and turn them into real pages (which is easily done with templates) or suppress them so they only show on the docs/glossary page and tooltips. This is a design preference, but suppressing the pages is easier by definition and we can add glossary term pages later on if we decide we want them.

There's also the option of moving the glossary to a directory that isn't picked up by the DocsPage model.

content/glossary

Still pretty close to the docs, this folder is still picked up by our gatsby-source-filesystem instance but not the DocsPage model, so it's safe to keep glossary entries here without them making pages. If we don't ever want individual pages for glossary entries, this is the simplest way to go about it.

rogermparent · 2020-05-23T01:18:33Z

I just realized that the glossary doesn't seem to have its own page in master, but the source file has data as if it were a page. Did we have one that broke in the past, or was that data just left over?

Either way, that's another design decision- we can either have a glossary page or only use the data for tooltips.

shcheklein · 2020-05-23T01:34:43Z

Excellent stuff @rogermparent !

Did we have one that broke in the past, or was that data just left over?

We never had a stand-alone page for it - see #542

Primarily, we need to figure out what directory to use for glossary entries and where we want the glossary entries to show up

Good question. My thoughts on this:

Let's move them into user-guide/basic-concepts/* (keep them hidden for now).

We can make it the first strep of solving the #947 and #542 - we'll have a section under the user guide with multiple pages, each page is a MD file with frontmatter that defines tooltip + some longer description that describes the concept in details.

Start expanding the concept descriptions as part of the #947 - btw, could be a good GSoD task /cc @jorgeorpinel

@rogermparent @jorgeorpinel WDYT?

src/gatsby/models/json-files/index.js

rogermparent · 2020-05-23T04:24:28Z

I need to bring that Gatsby child node factory I made in the Community Model into this one and the others- I didn't think of it until the codeclimate complaint.

Not necessarily related to this issue, but I think most everything's taken care of so I'll start on this branch and make a new PR if we merge this PR before I'm done with the refactor.

content/docs/user-guide/basic-concepts/import-stage.md

content/docs/user-guide/basic-concepts/dependency.md

package.json

We no longer use it anywhere.

This simple wrapper was introduced in the Community Model. I'm just using it in the other models now where applicable. `childNodeCreator` makes a function that acts like `createNode`, but adds in the parent ID of the current node calls `createParentChildLink` between the two nodes, and resolves when the prior two tasks do. I invoke the creators in the "parent" models' APIs so the Models that consume them get a nice function for creating child nodes. The consumer API calls that use this helper don't even need to destructure `actions` anymore, as making a child node is their only function most of the time.

content/docs/user-guide/basic-concepts/data-artifact.md

shcheklein · 2020-05-23T20:42:08Z

@rogermparent it has not been redeployed for some reason?

rogermparent · 2020-05-23T22:53:14Z

@shcheklein That's really weird, I'm not sure what happened there but I triggered a redeploy on Heroku and that fixed it for me. It deployed over the same URL at https://dvc-landing-build-time--fnpeae.herokuapp.com, but GitHub still marks the automatic deploy as inactive.

rogermparent · 2020-05-23T23:03:48Z

It works on the latest auto-deploy! Might've been some sort of caching issue, but unless it shows up again I'm willing to write it off as a fluke.

shcheklein · 2020-05-23T23:06:20Z

@rogermparent thanks! awesome stuff!

jorgeorpinel · 2020-05-25T21:27:06Z

content/docs/glossary.js

-    {
-      name: 'DVC Project',
-      match: [
-        'DVC project',
-        'DVC projects',
-        'project',
-        'projects',
-        'DVC repository',
-        'DVC repositories'
-      ],
-      desc: `
-Initialized by running \`dvc init\` in the **workspace** (typically in a Git
-repository). It will contain the
-[\`.dvc/\` directory](/doc/user-guide/dvc-files-and-directories) and
-[DVC-files](/doc/user-guide/dvc-file-format) created with commands such as
-\`dvc add\` or \`dvc run\`.
-      `
-    },


I think we missed migrating this one.

Whoops! Thanks for the catch!

Fixing it in #1358 🙂

Great, thanks!

per #1334 (review)

* cmd ref: update WebDAV remote info per #1187 (comment) * cmd ref: remove WebDAV remote info for #1355 * glossary: add back DVC project per #1334 (review) * Re-add abbrs within details This feature broke at some point in the past and all these were removed. The feature has since been fixed, so these are safe to re-add. * Re-add cache directory in add-files * Change "cache directory" to "cache" We don't have a tooltip match for "cache directory" * Re-add some missed abbrs * Revert grammar change I think I missed the correct context originally when editing it, and committed it because it looked like any other abbr addition. Co-authored-by: Jorge Orpinel <[email protected]>

rogermparent added 4 commits May 21, 2020 22:36

Add sourceNodes to the Gatsby APIs called by Models

30f84e4

Change glossary to ES5

d50ff3f

This lets it be used at build time without transpilation. We could also use JSON, but this change is smaller and easier.

Add Types for Glossary hook

5b2ee6b

shcheklein had a problem deploying to dvc-landing-build-time--h14bqc May 22, 2020 03:03 Failure

Explicitly add remark-rehype

d275fb5

shcheklein temporarily deployed to dvc-landing-build-time--fnpeae May 22, 2020 18:05 Inactive

rogermparent temporarily deployed to dvc-landing-build-time--h14bqc May 22, 2020 18:05 Inactive

Fix glossary entry for DVC cache

f9362a7

Remark dislikes special elements broken across lines halfway, so this is needed to get the link processsed.

rogermparent temporarily deployed to dvc-landing-build-time--fnpeae May 22, 2020 18:54 Inactive

rogermparent added 2 commits May 22, 2020 17:30

Merge branch 'master' of https://github.com/iterative/dvc.org into bu…

0f4fafb

…ild-time-tooltips

rogermparent had a problem deploying to dvc-landing-build-time--fnpeae May 22, 2020 21:41 Failure

Fix link line break in glossary yml

5d514fb

Remark doesn't like breaking specifically between `]` and `(` in a link, but this should be fine.

rogermparent had a problem deploying to dvc-landing-build-time--fnpeae May 22, 2020 21:57 Failure

Fix Community model

49cdc27

rogermparent had a problem deploying to dvc-landing-build-time--fnpeae May 22, 2020 23:02 Failure

rogermparent temporarily deployed to dvc-landing-build-time--fnpeae May 23, 2020 04:08 Inactive