Skip to content

refactor: remove mustache #361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 19, 2025
Merged

Conversation

araujogui
Copy link
Member

Description

Remove the mustache dep. I can't understand why replacing the script didn't work, the template gets rendered multiple times. but concatenation works 🤔

Validation

npx doc-kit generate -i doc/api/*.md -t web -o out/

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run node --run test and all tests passed.
  • I have check code formatting with node --run format & node --run lint.
  • I've covered new added functionality with unit tests if necessary.

@Copilot Copilot AI review requested due to automatic review settings July 18, 2025 00:36
@araujogui araujogui requested a review from a team as a code owner July 18, 2025 00:36
Copy link

vercel bot commented Jul 18, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
api-docs-tooling ✅ Ready (Inspect) Visit Preview Jul 18, 2025 1:32pm

Copy link
Contributor

@Copilot 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 removes the Mustache templating dependency and replaces it with a custom string replacement approach for HTML template processing. The change eliminates an external dependency while maintaining the same functionality for rendering Node.js documentation pages.

  • Replaced Mustache template rendering with manual string replacement for template variables
  • Removed the mustache package dependency from package.json
  • Modified HTML template structure to accommodate the new rendering approach

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.

File Description
src/generators/web/utils/processing.mjs Implemented custom string replacement logic to replace Mustache.render() functionality
src/generators/web/template.html Updated template syntax and structure for manual string replacement
package.json Removed mustache dependency and added debug flag to run script

@avivkeller
Copy link
Member

I can't understand why replacing the script didn't work, the template gets rendered multiple times

I had the same issue, that's why i used mustache. I plan to investigate further, when I get a chance.

@avivkeller
Copy link
Member

avivkeller commented Jul 18, 2025

Screenshot 2025-07-17 at 21 22 59 @araujogui Aha! The Shiki configuration contains several `` $` `` calls

@avivkeller
Copy link
Member

If you change it to .replace(..., () => theJS, it'll work

@avivkeller
Copy link
Member

We should probably change all instances of unknown code replacement, to prevent it from happening at any point

@araujogui
Copy link
Member Author

Screenshot 2025-07-17 at 21 22 59 @araujogui Aha! The Shiki configuration contains several $` calls

Nice catch 🎊 I put the replace back

@ovflowd
Copy link
Member

ovflowd commented Jul 18, 2025

@flakey5 we probably need to also update upstream PR on nodejs/node for the righ binary invocation, since now the binary is doc-kit (unrelated to this PR, but just wanted to use the opportunity)

@araujogui araujogui merged commit 93e0cbe into nodejs:main Jul 19, 2025
16 checks passed
@araujogui araujogui deleted the refactor/remove-mustache branch July 19, 2025 16:21
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.

5 participants