Skip to content

fix: improve cache handling and error recovery#2744

Open
sy-records wants to merge 4 commits into
docsifyjs:developfrom
sy-records:fix/embed
Open

fix: improve cache handling and error recovery#2744
sy-records wants to merge 4 commits into
docsifyjs:developfrom
sy-records:fix/embed

Conversation

@sy-records

Copy link
Copy Markdown
Member

Summary

Related issue, if any:

Close #2741

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

For any code change,

  • Related documentation has been updated, if needed
  • Related tests have been added or updated, if needed

Does this PR introduce a breaking change?

  • Yes
  • No

Tested in the following browsers:

  • Chrome
  • Firefox
  • Safari
  • Edge

@vercel

vercel Bot commented Jun 11, 2026

Copy link
Copy Markdown

@sy-records is attempting to deploy a commit to the Docsify Team on Vercel.

A member of the Team first needs to authorize it.

@vercel

vercel Bot commented Jun 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docsify-preview Ready Ready Preview, Comment Jun 11, 2026 5:43am

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes two rendering/cache edge cases in Docsify: memoization that previously failed to cache falsy/undefined results, and embed :include rendering that could stall or error when an embed fetch fails. It also adds coverage to prevent regressions.

Changes:

  • Fix cached(fn) to memoize falsy and undefined return values correctly.
  • Ensure failed embed fetches don’t block prerendering, and avoid accessing embedToken.links when no embed token was produced.
  • Add unit/integration tests for the above behaviors.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
test/unit/core-util.test.js Adds unit tests validating cached() memoizes falsy and undefined values.
test/integration/embed.test.js Adds an integration test ensuring a failed :include fetch does not block rendering.
src/core/util/core.js Updates cached() implementation to use key existence checks rather than truthiness.
src/core/render/embed.js Adds embed fetch error handling and adjusts prerender logic to tolerate missing embed tokens.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/integration/embed.test.js
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread src/core/render/embed.js
Comment thread src/core/render/embed.js Outdated

@paulhibbitts paulhibbitts left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sy-records , thanks very much for taking this on! I was unable to test this issue on Codesandbox (maybe file error checking?) so I created a repo on GitHub at https://github.com/paulhibbitts/docsify-v5-failed-embeds

Using the current RC and an unavailable embed, the loading of the site clearly failed:

Image

While using the PR Preview build the site was able to load successfully!
https://paulhibbitts.github.io/docsify-v5-failed-embeds/#/

Image

So testing results are looking good for me.🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants