fix: glossary recursion #857
Conversation
| * @param {String} type key denoting type/ grouping of markdown data, to help parse frontmatter | ||
| * @returns {Object|null} | ||
| */ | ||
| const getUsageMetaData = (frontmatter, type) => { |
There was a problem hiding this comment.
This function does two things. Should be two functions (or none).
| @@ -153,13 +148,29 @@ export const getGlossaryUsedLink = (slug, allGlossary) => { | |||
|
|
|||
| export const getGlossaryItemsUsedInRule = slug => { | |||
| const keys = [] | |||
There was a problem hiding this comment.
What are keys and slugs? Please give these meaningful names.
| * @param {String} type key denoting type/ grouping of markdown data, to help parse frontmatter | ||
| * @returns {Array<Object>} | ||
| */ | ||
| const getGlossaryUsagesInMarkdownData = (markdownData, type = 'rule') => { |
There was a problem hiding this comment.
Can you please stop using "glossary" as a synonym for "definition". The glossary is a list of definitions. Each rule has one glossary, containing one or more definitions. There are no glossaries inside of other glossaries. Some definitions have references to other definitions.
| * @returns {Array<Object>} | ||
| */ | ||
| const getGlossaryUsagesInMarkdownData = (markdownData, type = 'rule') => { | ||
| const usages = {} |
There was a problem hiding this comment.
That's not a meaningful variable name.
| @@ -153,13 +148,29 @@ export const getGlossaryUsedLink = (slug, allGlossary) => { | |||
|
|
|||
| export const getGlossaryItemsUsedInRule = slug => { | |||
There was a problem hiding this comment.
The logic for coming up with the glossary for a rule should not live in a render method. That stuff needs to be isolated. Quite possibly this glossaries-in-glossaries.json file shouldn't even exist, and all that info should be rolled up into a simple function that, you pass it the rule ID, and it returns an array of definitions IDs used in that rule.
| * Iterate through references in rules | ||
| */ | ||
| Object.keys(glossariesInRules).forEach(key => { | ||
| glossariesInRules[key].forEach(({ slug: s }) => { |
There was a problem hiding this comment.
What is "s"? Please give this a meaningful name.
| */ | ||
| Object.keys(glossariesInGlossaries).forEach(key => { | ||
| glossariesInGlossaries[key].forEach(({ glossarykey }) => { | ||
| if (!keys.includes(glossarykey)) { |
There was a problem hiding this comment.
It's hard to tell (because the code is hard to read), but it looks like this is including everything. Definitely this isn't recursively looking up definitions inside of other definitions.
|
I have worked out a POC to reduce this step into a gatsby preBootstrap script. That will be a refactor exercise. I will close this PR and re-open the relevant issues. |
glossaryterms that may have been referred with in a glossary and enumerates them in the glossary definitions for a rule.glossaryterms internally referred within aglossaryexists.Closes issue(s):
Pull Request Etiquette
When creating PR:
developbranch (left side).After creating PR:
Rule,DefinitionorChore.How to Review And Approve