-
Notifications
You must be signed in to change notification settings - Fork 143
add !STARTERCONF for demo elements #604
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
base: v3-main
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces comments throughout the codebase to mark demo mode–related code, imports, exports, and configuration for future removal or modification. No functional changes are made except for a minor adjustment in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant DemoComponents
participant DemoModeDrawer
User->>App: Access dashboard or home page
App->>DemoComponents: Render DemoAppSwitch, DemoMarketingBento, DemoWelcome
Note right of DemoComponents: Comments indicate these imports/usages are for demo mode and should be removed
App->>DemoModeDrawer: (If error DEMO_MODE_ENABLED)\nOpen DemoModeDrawer
Note right of DemoModeDrawer: Comments indicate this logic is for demo mode and should be removed
sequenceDiagram
participant User
participant App
participant LoginHint
User->>App: Access login page
App->>LoginHint: Render LoginEmailHint / LoginEmailOtpHint
Note right of LoginHint: Now always rendered in production, regardless of demo mode
✨ Finishing Touches
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 (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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: 1
🧹 Nitpick comments (5)
app/server/email.tsx (1)
19-22
: Server code should not rely onenvClient
sendEmail
executes exclusively on the server. UsingenvClient.VITE_IS_DEMO
couples a server pathway to the client env-schema, needlessly importing extra code and risking tree-shaking leaks.
Prefer the server-side schema (or a plainprocess.env
check) or remove the branch altogether since it is marked for deletion.- if (envClient.VITE_IS_DEMO) { + // keep the semantics while decoupling from client bundle + if (envServer.VITE_IS_DEMO /* or Boolean(process.env.VITE_IS_DEMO) */) { return; }When the demo flag is finally removed, the
if
block can be dropped entirely.app/features/auth/config.ts (1)
3-3
: Simplify the boolean assignment and silence the linterThe ternary is redundant (
Biome
already flagged this). A direct negation is clearer and avoids future “useless ternary” warnings.-export const AUTH_SIGNUP_ENABLED = envClient.VITE_IS_DEMO ? false : true; // !STARTERCONF [demoMode] Remove the envClient.VITE_IS_DEMO condition. You can +export const AUTH_SIGNUP_ENABLED = !envClient.VITE_IS_DEMO; // !STARTERCONF [demoMode] Remove this condition entirely when demo mode is dropped.This keeps the same behaviour (disabled in demo mode, enabled otherwise) with less code.
app/env/client.ts (1)
28-33
: Consider adding a deprecation comment to the schemaThe added
!STARTERCONF
note explains the upcoming removal, but once the env var is dropped, this whole block should disappear. You may avoid accidental resurrection by wrapping it in a/* DEMO-MODE ONLY – DELETE ME */
block or tracking it in aneslint-comments
rule so CI fails if it lingers.app/server/orpc.ts (1)
101-109
: SameenvClient
vs. server context concernThe middleware lives on the server yet checks
envClient.VITE_IS_DEMO
. Align this withenvServer
(orprocess.env
) to avoid bundling the client schema into server code, then drop the check once demo mode is gone.- if (envClient.VITE_IS_DEMO && procedure['~orpc'].route.method !== 'GET') { + if (envServer.VITE_IS_DEMO && procedure['~orpc'].route.method !== 'GET') {This keeps layering clean until the middleware is fully removed.
app/features/home/app/page-home.tsx (1)
20-24
: Placeholder comment but no TODO trackerThe comment signals “update with your content” yet there’s no TODO/HACK tag for automated tracking. Consider adding
// TODO(start-ui-cleanup): replace demo components
so search/CI can catch it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/e2e-tests.yml
(1 hunks)app/env/client.ts
(1 hunks)app/features/auth/config.ts
(1 hunks)app/features/dashboard/manager/page-dashboard.tsx
(2 hunks)app/features/demo/demo-app-switch.tsx
(1 hunks)app/features/demo/demo-marketing-bento.tsx
(1 hunks)app/features/demo/demo-mode-drawer.tsx
(1 hunks)app/features/demo/demo-welcome.tsx
(1 hunks)app/features/devtools/login-hint.tsx
(2 hunks)app/features/home/app/page-home.tsx
(2 hunks)app/lib/orpc/client.ts
(1 hunks)app/locales/ar/index.ts
(2 hunks)app/locales/en/index.ts
(2 hunks)app/locales/fr/index.ts
(2 hunks)app/locales/sw/index.ts
(2 hunks)app/providers.tsx
(2 hunks)app/server/auth.tsx
(1 hunks)app/server/email.tsx
(1 hunks)app/server/orpc.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
app/server/auth.tsx (1)
Learnt from: ivan-dalmet
PR: BearStudio/start-ui-web#532
File: src/server/config/oauth/providers/discord.ts:11-11
Timestamp: 2024-10-14T15:29:53.279Z
Learning: In `src/server/config/oauth/providers/discord.ts`, when defining the `zDiscordUser` schema, keep fields like `username` as nullable (e.g., `.nullish()`), as the Discord API might return null values despite the documentation stating otherwise.
app/features/home/app/page-home.tsx (1)
Learnt from: ivan-dalmet
PR: BearStudio/start-ui-web#532
File: src/features/auth/PageOAuthCallback.tsx:43-45
Timestamp: 2024-09-30T11:07:14.833Z
Learning: When suggesting changes to `useEffect` dependencies in React components, ensure that removing dependencies doesn't cause React Hook errors about missing dependencies.
app/features/devtools/login-hint.tsx (1)
Learnt from: ivan-dalmet
PR: BearStudio/start-ui-web#532
File: src/features/auth/OAuthLogin.tsx:57-60
Timestamp: 2024-10-14T15:29:19.311Z
Learning: In the `OAuthLoginButtonsGrid` component in `src/features/auth/OAuthLogin.tsx`, `OAUTH_PROVIDERS_ENABLED_ARRAY` is already filtered to include only enabled OAuth providers.
app/providers.tsx (2)
Learnt from: ivan-dalmet
PR: BearStudio/start-ui-web#532
File: src/features/auth/OAuthLogin.tsx:57-60
Timestamp: 2024-10-14T15:29:19.311Z
Learning: In the `OAuthLoginButtonsGrid` component in `src/features/auth/OAuthLogin.tsx`, `OAUTH_PROVIDERS_ENABLED_ARRAY` is already filtered to include only enabled OAuth providers.
Learnt from: ivan-dalmet
PR: BearStudio/start-ui-web#532
File: src/features/auth/PageOAuthCallback.tsx:43-45
Timestamp: 2024-09-30T11:07:14.833Z
Learning: When suggesting changes to `useEffect` dependencies in React components, ensure that removing dependencies doesn't cause React Hook errors about missing dependencies.
🧬 Code Graph Analysis (5)
app/features/auth/config.ts (1)
app/env/client.ts (1)
envClient
(24-54)
app/lib/orpc/client.ts (1)
app/env/client.ts (1)
envClient
(24-54)
app/server/auth.tsx (1)
app/env/client.ts (1)
envClient
(24-54)
app/features/devtools/login-hint.tsx (1)
app/env/client.ts (1)
envClient
(24-54)
app/providers.tsx (2)
app/features/demo/demo-mode-drawer.tsx (1)
useIsDemoModeDrawerVisible
(23-25)app/components/ui/sonner.tsx (1)
Sonner
(4-37)
🪛 Biome (1.9.4)
app/features/auth/config.ts
[error] 3-3: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
🪛 Checkov (3.2.334)
.github/workflows/e2e-tests.yml
[MEDIUM] 23-24: Basic Auth Credentials
(CKV_SECRET_4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Playwright E2E Tests
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (19)
app/features/demo/demo-app-switch.tsx (1)
1-1
: Comment addition looks finePurely adds a
!STARTERCONF
marker; no runtime impact.app/features/demo/demo-welcome.tsx (1)
1-1
: Marker comment acknowledgedNo code change apart from the starter-config note.
app/features/demo/demo-mode-drawer.tsx (1)
1-1
: No objectionsThe added annotation is non-functional; implementation remains intact.
app/features/demo/demo-marketing-bento.tsx (1)
1-1
: LGTMJust a starter-conf comment; nothing else modified.
app/features/home/app/page-home.tsx (1)
3-5
: Imports flagged for removal are still used belowLines 21-23 render the demo components, so deleting only the imports later will break the page. Either:
- Replace the components with real content before dropping the imports, or
- Guard their usage behind the same demo-flag until removal.
Documenting the intended sequence will help avoid accidental runtime errors.
app/features/dashboard/manager/page-dashboard.tsx (2)
1-3
: Well-placed import comments for demo mode cleanup.The comments clearly mark these demo component imports for removal during starter configuration cleanup. This approach provides clear guidance for future maintenance.
19-19
: Clear guidance for content replacement.The JSX comment effectively communicates that this section should be updated with actual content when removing demo mode, making the cleanup process more straightforward.
app/lib/orpc/client.ts (2)
7-7
: Accurate import comment for demo mode removal.The comment correctly identifies the demo mode drawer import for removal, consistent with the broader cleanup effort.
15-18
: Precise conditional logic marking.The comment accurately identifies the demo mode error handling block for removal, which aligns with the overall demo mode cleanup strategy.
app/locales/fr/index.ts (2)
10-10
: Comprehensive removal guidance for demo localization.The comment clearly indicates both the import and the file should be removed, providing complete guidance for demo mode cleanup.
23-23
: Consistent export marking.The export comment complements the import comment, ensuring complete removal of demo localization during cleanup.
app/locales/ar/index.ts (1)
10-10
: Consistent localization cleanup comments.The comments follow the same pattern as other locale files, ensuring consistent demo mode removal guidance across all localizations.
Also applies to: 23-23
app/server/auth.tsx (1)
59-62
: Precise demo mode condition marking.The comment accurately identifies the specific demo mode condition for removal, which aligns with the
VITE_IS_DEMO
environment variable removal noted inapp/env/client.ts
. This targeted approach preserves the development mode functionality while marking demo-specific code.app/locales/sw/index.ts (1)
10-10
: Clear and consistent demo mode marking.The comments clearly indicate which demo-related imports and exports should be removed during starter configuration cleanup. This follows a consistent pattern that will make future refactoring straightforward.
Also applies to: 23-23
app/features/devtools/login-hint.tsx (2)
13-14
: Logical preparation for demo mode removal.The comment correctly identifies the condition that should be removed when demo mode is eliminated. The AlertTitle change to always show "Dev mode" is logical since demo mode is being phased out.
Also applies to: 21-21
43-44
: Consistent approach across both hint components.The same logical changes are applied to both
LoginEmailHint
andLoginEmailOtpHint
components, maintaining consistency in the codebase.Also applies to: 51-51
app/locales/en/index.ts (1)
10-10
: Consistent demo mode marking across locales.The comments maintain the same clear pattern as other locale files, ensuring consistent guidance for demo mode cleanup across all language files.
Also applies to: 23-23
app/providers.tsx (2)
12-14
: Clear guidance for demo mode component removal.The comments clearly identify which imports and declarations should be removed when cleaning up demo mode, providing specific guidance for the refactoring process.
Also applies to: 17-17
26-27
: Thoughtful preservation of essential components.The comment correctly identifies that the
<Sonner />
component should be preserved while removing the demo mode-specific conditional logic. This ensures that important UI functionality (toast notifications) remains intact after demo mode cleanup.Also applies to: 29-30
EMAIL_SERVER: smtp://username:password@localhost:1025 | ||
EMAIL_FROM: Start UI <noreply@example.com> | ||
EMAIL_FROM: Example <noreply@example.com> |
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.
Avoid committing plaintext credentials & quote the new value
EMAIL_SERVER
still embedsusername:password
directly in the repo, which will be visible to every fork.EMAIL_FROM
now contains a space and angle-brackets; unquoted YAML can break on some parsers – quote it for safety.
- EMAIL_SERVER: smtp://username:password@localhost:1025
- EMAIL_FROM: Example <noreply@example.com>
+ # Prefer GitHub secrets for any credential-like value
+ EMAIL_SERVER: ${{ secrets.TEST_EMAIL_SERVER }}
+ # Quote to avoid YAML parsing surprises
+ EMAIL_FROM: "Example <noreply@example.com>"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
EMAIL_SERVER: smtp://username:password@localhost:1025 | |
EMAIL_FROM: Start UI <noreply@example.com> | |
EMAIL_FROM: Example <noreply@example.com> | |
# Prefer GitHub secrets for any credential-like value | |
EMAIL_SERVER: ${{ secrets.TEST_EMAIL_SERVER }} | |
# Quote to avoid YAML parsing surprises | |
EMAIL_FROM: "Example <noreply@example.com>" |
🧰 Tools
🪛 Checkov (3.2.334)
[MEDIUM] 23-24: Basic Auth Credentials
(CKV_SECRET_4)
🤖 Prompt for AI Agents
In .github/workflows/e2e-tests.yml at lines 23-24, remove the plaintext username
and password from the EMAIL_SERVER value to avoid exposing credentials in the
repo, and replace them with environment variables or secrets. Also, enclose the
EMAIL_FROM value in quotes to handle the space and angle brackets safely in YAML
parsing.
Add comments for easy start with the stater
Summary by CodeRabbit
Style
Bug Fixes
Chores