Compile partial HTML for embedding into Bump API docs #295
Conversation
✅ Deploy Preview for docs-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis update introduces a system for compiling Handlebars partial templates into HTML files as part of the build process. A new Node.js script ( Sequence Diagram(s)sequenceDiagram
participant Gulp as Gulp Build Process
participant CompileWidgets as compileWidgets Task
participant Script as compile-partial.js
participant FS as File System
participant Handlebars as Handlebars Engine
Gulp->>CompileWidgets: Start compileWidgets
CompileWidgets->>Script: Execute compile-partial.js (with partial name and context)
Script->>FS: Load helper JS files
Script->>Handlebars: Register helpers
Script->>FS: Load all partial templates
Script->>Handlebars: Register partials
Script->>FS: Load requested partial template
Script->>FS: Load optional JSON context
Script->>Handlebars: Compile template with context
Handlebars-->>Script: Rendered HTML
Script->>FS: Write HTML to assets/widgets directory
Script-->>CompileWidgets: Compilation complete
CompileWidgets-->>Gulp: Task complete
sequenceDiagram
participant HeaderPartial as header-content.hbs
participant Context as JSON Context (header-content.json)
participant CSS as CSS Variables
HeaderPartial->>Context: Read navigation, color, and config data
HeaderPartial->>CSS: Use --default-header-color if no custom color
CSS-->>HeaderPartial: Provide color value based on theme
HeaderPartial-->>HeaderPartial: Render navbar with resolved color
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Docstrings generation was requested by @JakeSCahill. * #295 (comment) The following files were modified: * `compile-partial.js` * `gulpfile.js`
|
Note Generated docstrings for this pull request at #296 |
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (2)
src/helpers/is-prerelease.js (1)
5-16:⚠️ Potential issueFix the version comparison logic.
There's a critical bug in the version comparison loop. The
elsestatement on line 14 will cause the function to returnfalseimmediately if the current version doesn't match the first version in the array, without checking the remaining versions.The logic should continue checking all versions before returning
false. Here's the fix:for (let i = 0; i < currentPage.component.versions.length; i++) { const version = currentPage.component.versions[i].version if (currentVersion === version) { if (currentPage.component.versions[i].prerelease === true) { return true } else { return false } - } else { - return false } } + return false🧰 Tools
🪛 Biome (1.9.4)
[error] 5-5: This code is unreachable
... because either this statement, ...
... this statement, ...
... this statement, ...
... this statement, ...
... or this statement will return from the function beforehand
(lint/correctness/noUnreachable)
context/header-content.json (1)
1-82:⚠️ Potential issueFix the JSON formatting issue.
The JSON structure and data look good for providing context to the header widget compilation. The color value
"#d73d23"appropriately matches the previously hardcoded value for consistency.However, there's a formatting issue on line 32. The closing brace is incorrectly positioned:
"page-header-data": { - "order": 2 } + "order": 2 + }
🧹 Nitpick comments (3)
src/ui.yml (1)
5-5: LGTM! Fix the formatting issue.The addition of
assets/**to static files is correct for including the compiled widget HTML files. However, please add a newline at the end of the file to fix the YAMLlint error.- assets/** + assets/**🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: no new line character at the end of file
(new-line-at-end-of-file)
compile-partial.js (1)
1-63: Consider adding JSDoc documentation.Since this script is part of the build pipeline, adding JSDoc comments would improve maintainability and help other developers understand its purpose and usage.
+/** + * Compile Handlebars partial templates to HTML files + * + * Usage: node compile-partial.js <partial-name> [context.json] + * + * @description This script compiles Handlebars partials into static HTML files + * for embedding in documentation. It registers all helpers and partials, + * then compiles the specified partial with optional JSON context. + */ + const fs = require('fs'); const path = require('path'); const Handlebars = require('handlebars');gulpfile.js (1)
31-51: Consider using asynchronous execution for better build performance.The current implementation uses
execSyncwhich blocks the build process. For better performance, especially with multiple partials, consider using the asynchronousexecwith proper error handling.-function compileWidgets (cb) { +async function compileWidgets (cb) { const partialsToCompile = [ { name: 'header-content', context: 'context/header-content.json' }, { name: 'footer' }, { name: 'head-styles' }, { name: 'head-scripts' }, ] try { - for (const { name, context } of partialsToCompile) { + const promises = partialsToCompile.map(({ name, context }) => { console.log(`🔧 Compiling partial: ${name}`) const cmd = context ? `node compile-partial.js ${name} ${context}` : `node compile-partial.js ${name}` - execSync(cmd, { stdio: 'inherit' }) - } + return new Promise((resolve, reject) => { + exec(cmd, { stdio: 'inherit' }, (err) => { + if (err) reject(err); + else resolve(); + }); + }); + }); + + await Promise.all(promises); cb() } catch (err) { cb(new Error(`Failed to compile Handlebars partials: ${err.message}`)) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
compile-partial.js(1 hunks)context/head.json(1 hunks)context/header-content.json(1 hunks)gulpfile.js(3 hunks)src/css/dark-mode.css(1 hunks)src/css/vars.css(1 hunks)src/helpers/is-prerelease.js(1 hunks)src/helpers/sort-components.js(1 hunks)src/partials/header-content.hbs(1 hunks)src/ui.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
src/ui.yml
[error] 5-5: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (6)
src/helpers/sort-components.js (1)
8-10: Excellent defensive programming practice!The guard clause effectively prevents runtime errors when the collection is null or undefined, which is particularly valuable in template compilation contexts where data sources might be incomplete.
src/css/dark-mode.css (1)
48-48: Good CSS variable addition for dark theme support.The
--default-header-colorvariable is well-named and appropriately uses--redpanda-1000for dark mode theming. The placement within the dark theme selector is correct.src/css/vars.css (1)
145-145: Perfect complement to the dark mode variable.The use of
--redpanda-800provides appropriate contrast for light themes, while maintaining consistency with the--redpanda-1000used in dark mode. The variable naming is consistent across both theme files.context/head.json (1)
1-9:Details
✅ Verification successful
Well-structured configuration for widget compilation.
The JSON structure is clean and supports the standalone widget compilation objective. The configuration includes appropriate settings for analytics, widget mode, and UI paths.
Please verify that the Google Analytics tracking ID
GTM-WB2CSV5is the correct one intended for this widget implementation:
🌐 Web query:
Is GTM-WB2CSV5 a valid Google Tag Manager container ID format and what are the best practices for using GTM IDs in standalone widgets?💡 Result:
Validity of GTM-WB2CSV5 as a Google Tag Manager Container ID
The format of a valid Google Tag Manager (GTM) container ID is GTM-XXXXXXX, where the "X" can be either letters or numbers, and typically the section after "GTM-" is a unique combination of six or seven alphanumeric characters[1][8][10]. Examples commonly provided include
GTM-XXXXXXorGTM-XXXXXXX.The ID
GTM-WB2CSV5matches this required format:
- Begins with "GTM-"
- Followed by a dash "-"
- Contains a string of 7 alphanumeric characters (WB2CSV5)
Therefore, GTM-WB2CSV5 is a valid Google Tag Manager container ID format[1][8][10].
Best Practices for Using GTM IDs in Standalone Widgets
When integrating GTM IDs in standalone widgets, consider these best practices to ensure robust and secure implementation:
1. Use Unique Containers for Isolated Environments
- If the widget is used on multiple independent sites or embedded in third-party environments, consider creating a dedicated GTM container for the widget. This prevents conflicts with host site tags and simplifies management[1][9].
2. Document Container Usage
- Always document which GTM container ID is used by which widget and where it is deployed. Good documentation assists with troubleshooting and maintenance, especially when you have multiple containers or are working in cross-team environments[7].
3. Include Versioning and Tag Names
- Use a container version ID along with the container ID to track changes and facilitate troubleshooting. This practice is particularly useful when the widget evolves over time[7].
- Set a custom tag name parameter (e.g., tag_name) where possible for clarity on which event originates from the widget versus the host site[7].
4. Prevent Data Layer Conflicts
- If your widget pushes to the GTM dataLayer, consider using a uniquely named data layer object to avoid clashes with the host site's dataLayer. You can rename the data layer parameter in your GTM snippet (e.g.,
myWidgetDataLayerinstead of the defaultdataLayer), and reference this throughout your widget code[6].5. Validate GTM Implementation
- Always verify that GTM is loading and firing as expected within your widget, ideally by inspecting the source code or using browser developer tools to ensure the correct container ID is called[3][5].
6. Avoid Exposing Sensitive Container Info
- Do not expose your GTM container ID or other sensitive information in public JavaScript beyond what is needed for GTM to function. This reduces the risk of misuse.
7. Use Environment-Specific Containers for Testing
- For development and staging, use separate GTM containers or environments to prevent test data from polluting production analytics[7].
Summary Table: Key Points
Practice Recommendation Container ID Format GTM-XXXXXXX(letters/numbers; e.g.,GTM-WB2CSV5is valid)Widget Isolation Use a dedicated GTM container for standalone widgets deployed on external sites Documentation Keep records of all GTM container IDs used and their deployment locations Container Versioning Include versioning for containers to track updates Custom Tag Names Use custom parameters for tracking tag sources Data Layer Naming Use a unique data layer name to prevent conflicts with host website Implementation Validation Test widget with browser dev tools to confirm correct GTM loading and event firing Security Limit public exposure of GTM IDs and configurations Use of Environments Separate GTM containers/environments for production, staging, and development Following these practices will make your GTM-powered widgets more reliable, manageable, and secure when deployed independently or within third-party sites.
Citations:
- 1: https://analytify.io/google-tag-manager-id/
- 2: https://support.google.com/tagmanager/answer/12326985
- 3: https://www.brightspot.com/documentation/brightspot-integrations-guide/validating-google-tag-manager-code-makes-call-to-google-tag-manager
- 4: https://www.simoahava.com/analytics/check-container-id-with-eventcallback/
- 5: https://www.optimizesmart.com/check-if-google-tag-manager-is-working/
- 6: https://developers.google.com/tag-platform/tag-manager/datalayer
- 7: https://measuremindsgroup.com/google-tag-manager-best-practices
- 8: https://help.sertifier.com/google-measurement-id-and-google-tag-manager-id
- 9: https://support.google.com/tagmanager/answer/14842164
- 10: https://communicocollege.com/google-tag-manager-1265
GTM Container ID Validated – Ready to Merge
The container ID
GTM-WB2CSV5conforms to theGTM-XXXXXXXformat and is valid for standalone widget use. No changes are required in head.json.Optional best practices for future iterations:
- Use a dedicated GTM container for isolated widget deployments to avoid host-site tag conflicts.
- Document which container ID is used by each widget and where it’s embedded.
- Rename the dataLayer (e.g.
myWidgetDataLayer) to prevent collisions with host sites.- Employ environment-specific containers (dev/staging/production) to keep test data separate.
- Verify GTM load and event firing via browser dev tools before release.
src/helpers/is-prerelease.js (1)
2-2: Good improvement to the null check!Explicitly returning
falseinstead ofundefinedimproves type consistency and makes the function's behavior more predictable.gulpfile.js (1)
128-128: Verify the build pipeline integration.The
compileWidgetstask is correctly positioned betweencopyRapidocandbuildTask, ensuring partials are compiled before the main build. This integration looks appropriate for the intended workflow.
Docstrings generation was requested by @JakeSCahill. * #295 (comment) The following files were modified: * `compile-partial.js` * `gulpfile.js` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This pull request adds the initial support for integrating Bump in embedded mode. In order to support this mode, we need to compile the header and footer templates with some mocked data (in the
context/directory in the JSON files) so that we can download them and include them in the proxied Bump content: redpanda-data/docs-site#113We're continuing to support the Rapidoc integration until we go live with Bump.
A follow-up PR will be submitted to remove Rapidoc and apply final changes required to go live with Bump.
Handlebars partials compilation and integration:
compile-partial.jsto compile Handlebars partials into HTML files. The script registers helpers, loads partials, reads optional JSON context, and writes the compiled output to a designated directory.gulpfile.jsto include a newcompileWidgetsfunction for compiling specific Handlebars partials (header-content,footer,head-styles,head-scripts) during the build process. This function uses theexecSyncmethod to execute thecompile-partial.jsscript.compileWidgetstask into thebundle:buildseries ingulpfile.jsto ensure partials are compiled as part of the build pipeline.UI configuration and styling updates:
context/head.jsonandcontext/header-content.jsonfiles to provide JSON context for Handlebars partials. These files include site metadata, component information, and page attributes for dynamic rendering. [1] [2]src/css/dark-mode.cssandsrc/css/vars.cssto introduce--default-header-colorfor consistent header styling across themes. [1] [2]header-content.hbspartial to use the new--default-header-colorvariable for the navbar background color when no specific page-header color is defined.Helper function improvements:
src/helpers/is-prerelease.jsto handle cases wherecurrentPageis null or undefined, returningfalseinstead of potentially causing errors.src/helpers/sort-components.jsto return an empty array when the input collection is null or undefined, ensuring robustness.Miscellaneous updates:
assets/**to thestatic_filesconfiguration insrc/ui.ymlto include compiled Handlebars partials in the build output.