Issue with v1.6.4 vs v1.7.0

[jsmartfo]

I am seeing a error kicked into the console that breaks the page, it was working fine with v1.6.4 when I migrate to v1.7.0 I see the below:

ReDoc initialized! redoc.min.js:15 EXCEPTION: Uncaught (in promise): TypeError: Cannot read property 'split' of null TypeError: Cannot read property 'split' of null at t.exports (https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:25:29784) at t.headingOpenRule (https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:23:13881) at r.render (https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:41:12080) at i.render (https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:41:8399) at t.renderMd (https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:23:14412) at t.preprocess (https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:8:6678) at t.init (https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:8:6377) at https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:8:5848 at t.invoke (https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:48:8393) at Object.onInvoke (https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:11:2457) at t.invoke (https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:48:8333) at n.run (https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:48:5258) at https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:48:2869 at t.invokeTask (https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:48:9070) at Object.onInvokeTask (https://rebilly.github.io/ReDoc/releases/latest/redoc.min.js:11:2357)

BTW, fantastic viewer really enhances the definitions.

[RomanHotsiy]

Hey @jsmartfo. Could you provide me with a link to your spec?

[jsmartfo]

The spec is an internal one that I cannot publish yet, I am more that happy to debug -- let me try and setup and work out what is happening

[RomanHotsiy]

ok, if you can't find the exact issue it would be great if you come up with a minimal reproducible spec.

Most likely the issue is connected to the second-level (##) markdown headers in the description field of swagger

[jsmartfo]

Yep will do, either way will get it across.

May run into next week as I have some commitments this week/end.

[jsmartfo]

You're correct, it's an error when second level heading is in desc but you don't have a first level heading.

It appears slugify is throwing the error because it cannot accept null for this.currentHeading

  headingOpenRule(tokens, idx) {
    if (tokens[idx].hLevel > 2 ) {
      return this._origRules.open(tokens, idx);
    } else {
      let content = tokens[idx + 1].content;
      if (tokens[idx].hLevel === 1 ) {
        this.firstLevelHeadings.push(content);
        this.currentHeading = content;
        let contentSlug = slugify(content);
        return `<h${tokens[idx].hLevel} section="section/${contentSlug}">` +
          `<a class="share-link" href="#section/${contentSlug}"></a>`;
      } else if (tokens[idx].hLevel === 2 ) {
        this.secondLevelHeadings.push(this.currentHeading + `/` + content);
        let contentSlug = slugify(this.currentHeading) + `/` + slugify(content);
        return `<h${tokens[idx].hLevel} section="section/${contentSlug}">` +
          `<a class="share-link" href="#section/${contentSlug}"></a>`;
      }
    }
  }

let contentSlug = slugify(this.currentHeading) +/+ slugify(content);

The line above is the culprit as this.currentHeading is null, if we look at the if check for hLevel === 1, https://github.com/Rebilly/ReDoc/blob/master/lib/utils/md-renderer.ts#L53

We assign this.currentHeading = content;

It's seems to be null for hLevel === 2 (if you don't have a level 1 prior) - not sure how you wish to proceed? safe guard against this or tell people to use header order correctly 😛

I can do a PR, please let me know how you want to proceed.

[RomanHotsiy]

I believe safe guard would be better.
Also I think we should append such headers (second level without corresponding first level) to this.firstLevelHeadings instead of this.secondLevelHeadings so they will behave as first-level. I believe it does make sense.

If you think so you can proceed with PR. This part of code is not covered by tests so do not worry about it. I will add tests myself asap

Thanks

[jsmartfo]

Just wondering what would be the issue leaving their choice of header and not altering it under the covers?

Something like below, we still push into the heading arrays as needed, but the h1 and h2 content/structure is the same - but I maybe missing the reason for the difference added within the two if checks (if it was for uniqueness I can append the heading level also)

  headingOpenRule(tokens, idx) {
    if (tokens[idx].hLevel > 2 ) {
      return this._origRules.open(tokens, idx);
    } else {
      let content = tokens[idx + 1].content;
      this.currentHeading = content;
      let contentSlug = slugify(content);
      if (tokens[idx].hLevel === 1 ) {
        this.firstLevelHeadings.push(content);
      } else if (tokens[idx].hLevel === 2 ) {
        this.secondLevelHeadings.push(content);
      }
      return `<h${tokens[idx].hLevel} section="section/${contentSlug}">` +
          `<a class="share-link" href="#section/${contentSlug}"></a>`;
    }
  }

[RomanHotsiy]

@jsmartfo h2 content should contain parent heading reference so it can be placed in correct place in menu. Check out: https://github.com/Rebilly/ReDoc/blob/master/lib/services/menu.service.ts#L252

[jsmartfo]

I will get something across to you as soon as I can, have a couple of hoops to jump through at work first.

[RomanHotsiy]

Fixed on v1.8.0
Plz confirm and reopen in case it's not fixed