[WIP] task/issue-275 Create Extensible Menus Query

[hutchgrant]

Related Issue

Resolves #275

Summary of Changes

1. Begins to address (query results are not deterministic #271) which requires deterministic queries through a single menu based query
2. Removes need for shelfList to use json file
3. Uses markdown-toc to generate a table of contents from every page flagged with front-matter variable linkheadings: true
4. Added markdown front-matter globals for menu, index, linkheadings which are added to our graph via the graph lifecycle
5. if linkheadings is flagged true, during graph lifecycle(as markdown file is already being read) we will parse the file for all headings, add it to our graph for that specific page, into an array we will use for our shelf later.
6. the menu front-matter is used to specifically define a menu to use with any given markdown file e.g. navigation or side
7. I've modified our graphql type defs to add link and menu
8. I've added filtering to the query to only display listing for specific menus. e.g. side or navigation
9. I've modified our resolver to instead look through graph for any pages with the corresponding menu name
10. as per 9, I've also made the resolver check if a file has been flagged listheadings if so, it will loop through the tableOfContents variable for that page, and create a list of children for that specific link.

TODO

1. shelf is not rendering the results correctly at the moment, this is important and is the next step, but the data however is being received correctly for all menus
2. fix broken collapse/render that was dependent on previous json file

Note

At the moment this WIP. Data is received correctly, for all menus, only need to render it.

[thescientist13]

Oh..., maybe i missing something, but I thought removing the require calls from the shelf was just a matter of enabling the ability to use import().

I think this feature you are proposing here should really start with an issue first. Might be helpful to lead with proposing something via that workflow rather than just jumping into a PR, especially since this is also pulling in elements of #274.

I was really only expecting #275 to be import() usage, tbh...

[hutchgrant]

I thought we were removing need for the menu json file altogether. This way it's done on a page by page basis determines what page should be in what menu, and if necessary, it will add all headings as a table of contents within the graph for the page.

In fact, your code started to do this with a seperate query within the shelf but you never utilized it. Now it's being utilized.

https://github.com/ProjectEvergreen/greenwood/pull/285/files#diff-a74a98b76cd88ec34d638df72cd58122L38

[hutchgrant]

This line is what I'm referring to when I resolved 275 and started 271, using my method. You knew we were going to use a query here. You started using the query, but only printed it out to console and never fully implemented it. I thought the require was only temporary because the query wasn't completed. Now it is completed.

[thescientist13]

Well the details of #275 are specifically in reference to the fact that we shouldn't be using NodeJS APIs (require) in browser code, where we should be using Browser APIs (import()) instead.

This is only permitted because webpack allows it, but browser code should only be using browser APIs, like import / export

I think per something I saw in #265 , this would require additional Babel configuration. I can rename the issue if it helps clarify the intent but that was the outcome I was looking for from that issue.

---

For this PR then and as with most (new) things, I still recommend that it all new ideas / enhancements goes through an issue workflow first ideally, to allow for technical design review / decision and share code snippets and APIs. Given the amount of moving parts here, I think this warrants it and then the issue links and branch names can be reflective of the intended scope / issue.

[hutchgrant]

You specifically put a query because the idea is to query our menu through graphql not import or require. There's no need to import or require that json file because its redundant(and therefore being removed) with a working graphql query. That's the point I think you're missing here. Read the rest of your code for the component.

No json file, no import, no require.

[thescientist13]

I understand the point, I wrote the issue. 😉

like I said, I'll just rename the issue because the main point was supporting import(), regardless of the use case. You make it seem like no one could ever need to use it, which is shortsighted IMO. Also because require never should have been used in the first place.

My opinion on issues preceding PRs still stands and would / should apply here though. Keep #275 for import() and make an issue for what you are suggesting here instead.

[hutchgrant]

closing this issue and i'll resubmit it