-
Notifications
You must be signed in to change notification settings - Fork 4
ENG-803 - Add v to roam settings #391
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
ENG-803 - Add v to roam settings #391
Conversation
…tion - Added `getVersion` and `getBuildDate` functions to retrieve version and build date from package.json, improving build metadata handling. - Updated the compile function to include version and build date in the global window object. - Refactored Settings component to display version and build date in the UI, enhancing user awareness of the application version.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughAdds build-time injection of version and build date via esbuild, exposes them on window, introduces a runtime utility to read these values, and updates the Settings UI to show version/build date while adjusting the feedback button layout. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Build as compile.ts (esbuild)
participant Win as window
participant App as Settings.tsx
participant Util as getVersion.ts
Dev->>Build: Run build
Build->>Build: Read package.json (version)<br/>Get current date (YYYY-MM-DD)
Build->>Win: Define __DISCOURSE_GRAPH_VERSION__<br/>Define __DISCOURSE_GRAPH_BUILD_DATE__
note over Build,Win: Values embedded at bundle time
App->>Util: getVersionWithDate()
Util->>Win: Read __DISCOURSE_GRAPH_VERSION__ / __DISCOURSE_GRAPH_BUILD_DATE__
Util-->>App: { version, buildDate }
App->>App: Render footer with version/date and feedback button
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/roam/scripts/compile.ts (1)
18-20: Make build date reproducible (SOURCE_DATE_EPOCH support)Support deterministic builds while keeping the current default.
-const getBuildDate = (): string => { - return new Date().toISOString().split("T")[0]; // YYYY-MM-DD format -}; +const getBuildDate = (): string => { + const epoch = process.env.SOURCE_DATE_EPOCH; + const d = epoch ? new Date(Number(epoch) * 1000) : new Date(); + return d.toISOString().split("T")[0]; // YYYY-MM-DD (UTC) +};apps/roam/src/components/settings/Settings.tsx (1)
247-249: Avoid duplicate calls and add a11y titleCall
getVersionWithDate()once and reuse; add a helpful title.- <span className="text-xs text-gray-500"> - v{getVersionWithDate().version}-{getVersionWithDate().buildDate} - </span> + <span className="text-xs text-gray-500" title={`Version ${version} (${buildDate})`}> + v{version}-{buildDate} + </span>Add (outside the selected lines), near the top of
SettingsDialogbeforereturn:const { version, buildDate } = getVersionWithDate();apps/roam/src/utils/getVersion.ts (1)
1-7: Consider avoiding global Window augmentation for narrow fieldsTo reduce coupling (per maintainers’ preference), consider switching to define-injected module constants (e.g.,
__DG_VERSION__,__DG_BUILD_DATE__) instead of augmentingWindow. If you keep globals, this is fine, but it does slightly pollute the global surface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/roam/scripts/compile.ts(2 hunks)apps/roam/src/components/settings/Settings.tsx(2 hunks)apps/roam/src/utils/getVersion.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-08T14:51:55.299Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#0
File: :0-0
Timestamp: 2025-07-08T14:51:55.299Z
Learning: DiscourseGraphs maintainers prefer not to add narrowly typed definitions for individual `process.env` keys to avoid unnecessary coupling.
Applied to files:
apps/roam/scripts/compile.ts
🔇 Additional comments (3)
apps/roam/scripts/compile.ts (1)
165-166: I wasn’t able to confirm whether thedefineblock in apps/roam/scripts/compile.ts actually contains the twowindow.__DISCOURSE_GRAPH_*__entries or if abanneroption is already set in the esbuild config. Please manually verify that:
- The
definemapping includes"window.__DISCOURSE_GRAPH_VERSION__": `"${getVersion()}"`, "window.__DISCOURSE_GRAPH_BUILD_DATE__": `"${getBuildDate()}"`- There is no existing
banner: { js: … }property in the same build options object.apps/roam/src/components/settings/Settings.tsx (2)
28-28: LGTM: import placement and naming
234-245: LGTM: feedback button container repositioningLayout change is safe; behavior unchanged.
…ency - Updated `getVersion` function to accept a root parameter, enhancing flexibility in reading package.json. - Changed default return values to "-" for better clarity in case of errors. - Enhanced `getVersionWithDate` to handle server-side rendering by returning default values when window is undefined.
- Updated `getVersion` function to remove the root parameter, simplifying its usage by directly reading from the current working directory. - Improved error handling with a warning for failed attempts to read version from package.json. - Adjusted the compile function to reflect the changes in `getVersion`, ensuring consistent version retrieval.
Summary by CodeRabbit
New Features
UI/Style
Chores