Skip to content

fix: handle failed embed fetches and falsy cached() returns#2741

Open
Diawhiz wants to merge 3 commits into
docsifyjs:developfrom
Diawhiz:develop
Open

fix: handle failed embed fetches and falsy cached() returns#2741
Diawhiz wants to merge 3 commits into
docsifyjs:developfrom
Diawhiz:develop

Conversation

@Diawhiz
Copy link
Copy Markdown

@Diawhiz Diawhiz commented Jun 5, 2026

  • embed.js: add error handler to get().then() so a failed :include fetch calls next('') instead of silently stalling page rendering
  • core/util/core.js: use instead of in cached() so functions that legitimately return falsy values ('' or 0) have their results stored and reused correctly

Summary

Related issue, if any:

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

- embed.js: add error handler to get().then() so a failed :include
  fetch calls next('') instead of silently stalling page rendering
- core/util/core.js: use  instead of  in
  cached() so functions that legitimately return falsy values ('' or 0)
  have their results stored and reused correctly
@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 5, 2026

@Diawhiz is attempting to deploy a commit to the Docsify Team on Vercel.

A member of the Team first needs to authorize it.

@Diawhiz
Copy link
Copy Markdown
Author

Diawhiz commented Jun 5, 2026

embed.js: A failed: include fetch would silently stall page rendering because the rejected promise had no error handler. Added a rejection handler that calls next('') so rendering continues gracefully on fetch failure.

core/util/core.js: cached() used a truthy check (hit ||) which caused functions returning falsy values like '' or 0 to bypass the cache on every call. Changed to hit !== undefined so falsy-but-valid results are stored and reused correctly.

Copy link
Copy Markdown
Author

@Diawhiz Diawhiz left a comment

Choose a reason for hiding this comment

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

embed.js: A failed: include fetch would silently stall page rendering because the rejected promise had no error handler. Added a rejection handler that calls next('') so rendering continues gracefully on fetch failure.

core/util/core.js: cached() used a truthy check (hit ||) which caused functions returning falsy values like '' or 0 to bypass the cache on every call. Changed to hit !== undefined so falsy-but-valid results are stored and reused correctly.

Copy link
Copy Markdown

Copilot AI left a comment

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 aims to fix two bug scenarios in Docsify’s rendering pipeline: (1) avoid stalled rendering when embedded :include fetches fail, and (2) ensure cached() correctly memoizes legitimate falsy return values (e.g., '', 0).

Changes:

  • Updated cached(fn) to treat cached falsy values as hits by checking hit !== undefined instead of using hit || ....
  • (Per PR title/description) Intended to add rejection handling for embed fetches, but that change does not appear to be present in the current code.

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

Comment thread src/core/util/core.js
const key = isPrimitive(str) ? str : JSON.stringify(str);
const hit = cache[key];
return hit || (cache[key] = fn(str));
return hit !== undefined ? hit : (cache[key] = fn(str));
@sy-records sy-records added the wait for information something is not clear, waiting for the author of the issue/pr label Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wait for information something is not clear, waiting for the author of the issue/pr

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants