Docfx: Cyclic reference dependency chain does not cross type boundary

[yufeih]

Right now the dependency chain reported from cyclic reference does not pass across type boundary. E.g, uid reference chain does not include markdown inclusion chain. This can sometimes be confusion. This issue tracks this minor known issue.

[herohua]

Per today's discussion, we may have two ways to handle detection on cyclic reference for different types.

Method 1: use separate stacks to track cyclic reference for different types
Pros:

• one stack needs to handle its own type only, the interface of that tracking stack would be quite stable

Cons:

• impossible to print out the whole logic call stack if references cross types, which may make error message less understandable and hard to take actions
• (minor) may need to go one level deeper before detection on cyclic reference finds a violation

Method 2: use one single stack to track cyclic reference across all types
Pros:

• possible to print out whole call stack

Cons:

• use a context to handle different types of references, but they may be on different level (property vs. file)
• need extra logic to handle "rebase"

Decision:

• keep using current separate stacks approach
• revisit the problem if
  
  
  
  • any new kind of dependency becomes a requirement
  
  
  • the lack of entire stack in error message becomes a real blocking issue
  
  

@OsmondJiang @yufeih @live1206 Let me know if anything is missing or incorrect in this note. Thanks!

cc @fenxu @superyyrrzz @partychen @mingwli

[herohua]

We need to finish using absolute path as intermediate data model first.

[herohua]

4078

[yufeih]

This is a hypothetical problem and does not exist in practice:

uid: a
summary: @b?displayProperty=summary
uid: b
summary: @a?displayProperty=summary

We simply shouldn't use the marked content of displayProperty as output link text, because that would very likely produce an incorrect HTML,

Given:

uid: a
summary: this is a markdown [link](https://docs.com)
description: @a?displayProperty=summary

If we used marked content of a.summary, a.description would be <a>this is a markdown <a></a></a>, which is an incorrect HTML.

I would rather defined displayProperty as the unprocessed content for internal references, in the above example, a.description simply becomes <a>this is a markdown [link](https://docs.com)</a>

[herohua]

let's check whether current data (e.g. .NET input files) has such scenario. based on current understanding, it seems we don't have this scenario.

[herohua]

It would be hard to imagine there is real case of @uid?displayProperty=summary which would eventually be rendered into <a href={link}>{summary}</a>. As a matter of fact, v2 does not handle this well either and no issue has been reported till now.

As a result, we'd like to limit the list of allowed displayProperty to only contain the properties of plain text (no markdown is allowed). One follow-up item is to implement a check and ban usages beyond the allowed list.

@yufeih @OsmondJiang @fenxu @live1206 @superyyrrzz @928PJY Please review above comment and let me know if anything is missing. Thanks!