gnab / remark

A simple, in-browser, markdown-driven slideshow tool.
http://remarkjs.com
MIT License
12.68k stars 856 forks source link

The new 0.15.0 release has broken all our sites on GitHub Pages #613

Closed rberlind closed 4 years ago

rberlind commented 4 years ago

The new release has broken our GitHub Pages that use Remark.js such as https://hashicorp.github.io/field-workshops-terraform/slides/azure/terraform-cloud/#1, https://hashicorp.github.io/field-workshops-vault/slides/multi-cloud/vault-oss/#1, and https://hashicorp.github.io/field-workshops-consul/slides/multi-cloud/consul-oss/#1 which had all been using https://remarkjs.com/downloads/remark-latest.min.js. After the release, those pages all came up blank. I should note that we are also using the Story template, https://github.com/xaprb/story

I have determined that using https://remarkjs.com/downloads/remark-0.14.0.min.js instead restores the websites.

If you try some of the 3 URLs I gave above, they might work, because we are transitioning to pinning the versions to 0.14.0

You can see source code for the Nomad workshop in https://github.com/hashicorp/field-workshops-nomad which pulls assets from https://github.com/hashicorp/field-workshops-assets, both of which are public repos.

Thanks Roger Berlind

Paula-Moraga commented 4 years ago

It also broke my R Markdown presentations. Many thanks for posting your solution!

callalilychen commented 4 years ago

I have the same issues with https://remarkjs.com/downloads/remark-latest.min.js. My slides works with https://remarkjs.com/downloads/remark-0.14.0.min.js

CrossR commented 4 years ago

I have also had this issue, but only on slide sets that have count: false on the title slide. Removing that made my slides visible again.

peterj commented 4 years ago

Hi! Sorry for the late response. I am working on reverting the change to .latest.min.js to still point to the 0.14.0.

peterj commented 4 years ago

@rberlind I looked at the repos you referenced and it seems like the count: false is the offending property that broke (just like @CrossR is seeing as well). I'll dig deeper to see why this is happening.

peterj commented 4 years ago

I was able to write a failing unit test + have a fix for it. From my tests, it looks like the issue repros only if the first slide has count: false - we have a unit test that was passing when the count:false is not on the first slide. However, adding count: false to the first slide, fails.

Let's assume we have the following slides:

count:false

Slide A
---
Slide B

When we parse the slides we check if the slide is included (based on the classes) and if it's not a layout slide as well as if it should be counted. The code inside the if statement does not execute, so the slides.byNumber[slideNumber] is undefined

if (slideIsIncluded && slide.properties.layout !== 'true' && slide.properties.count !== 'false') {
      slideNumber++;
      slides.byNumber[slideNumber] = [];
}

Later though, we call slides.byNumber[slideNumber].push(slideViewModel); and that's what's causing the issue. The fix would be to do a check before we call the .push (i.e. if we don't count the slides, there's no need to add it to the array).

rberlind commented 4 years ago

Thanks @peterj : Just to make sure, I understand, it sounds like you have fixed this problem and merged into the develop branch, but have not yet issued a new release or upated the 0.15.0 release. Is that correct?

Is it also correct that using https://remarkjs.com/downloads/remark-latest.min.js would still have the problem until a new release is issued?

peterj commented 4 years ago

Correct.

I made a change yesterday to remark-latest.min.js, so it's the 0.14.1 version, so the latest should work (but it's not going to be 0.15.0).

I realized that we do need much more testing probably - this issue you and others experiences was introduced 2 years ago, so I am afraid there might be more issues like that (I hope there won't be though :)).

peterj commented 4 years ago

It might make more sense to do a pre-release for 0.15.1 and ask people to try it out first to make sure there are no breaking issues like this one was.

rberlind commented 4 years ago

I'm confused because when I look at https://github.com/gnab/remark/releases, I don't see 0.14.1.

peterj commented 4 years ago

Ah, I assumed the release was created for it, but apparently not.

On Tue, Jan 28, 2020 at 10:43 Roger Berlind notifications@github.com wrote:

I'm confused because when I look at https://github.com/gnab/remark/releases, I don't see 0.14.1.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gnab/remark/issues/613?email_source=notifications&email_token=ACURJ3EKDRNUQIPKF6GMF2LRAB4DTA5CNFSM4KI2FD4KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKEN53I#issuecomment-579395309, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACURJ3GNGC44GFXAPSM7ZF3RAB4DTANCNFSM4KI2FD4A .