add !STARTERCONF for demo elements #604
Conversation
WalkthroughAdds optional VITE_IS_DEMO env schema with boolean transform; updates various files with starter-config comments; introduces DemoWelcome component; adjusts devtools login hint title text; updates dashboard/home to annotate demo content; tweaks locales (adds fr:user export, comments on demo entries); changes GitHub Actions EMAIL_FROM value. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
app/server/email.tsx (1)
19-22: Server code should not rely onenvClient
sendEmailexecutes exclusively on the server. UsingenvClient.VITE_IS_DEMOcouples 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.envcheck) 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
ifblock can be dropped entirely.app/features/auth/config.ts (1)
3-3: Simplify the boolean assignment and silence the linterThe ternary is redundant (
Biomealready 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
!STARTERCONFnote 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-commentsrule so CI fails if it lingers.app/server/orpc.ts (1)
101-109: SameenvClientvs. 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 componentsso 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
!STARTERCONFmarker; 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_DEMOenvironment 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
LoginEmailHintandLoginEmailOtpHintcomponents, 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
ivan-dalmet
force-pushed
the
v3-main
branch
2 times, most recently
from
cf02ee7 to
cbaf95e
Compare
ivan-dalmet
force-pushed
the
v3/starterconf-comments
branch
from
f6c12aa to
a8867a1
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/e2e-tests.yml (1)
24-24: Cosmetic change is fine, but past review concerns remain unaddressed.The sender name change from "Start UI" to "Example" is acceptable. However, the previously flagged issues still apply:
- The value should be quoted to handle spaces and angle brackets safely in YAML
- Line 23's plaintext credentials remain exposed
These concerns were already documented in the previous review and don't need to block this cosmetic change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/workflows/e2e-tests.yml(1 hunks)src/env/client.ts(1 hunks)src/features/auth/config.ts(1 hunks)src/features/dashboard/manager/page-dashboard.tsx(2 hunks)src/features/demo/demo-app-switch.tsx(1 hunks)src/features/demo/demo-marketing-bento.tsx(1 hunks)src/features/demo/demo-mode-drawer.tsx(1 hunks)src/features/demo/demo-welcome.tsx(1 hunks)src/features/devtools/login-hint.tsx(2 hunks)src/features/home/app/page-home.tsx(2 hunks)src/lib/orpc/client.ts(1 hunks)src/locales/ar/index.ts(2 hunks)src/locales/en/index.ts(2 hunks)src/locales/fr/index.ts(2 hunks)
✅ Files skipped from review due to trivial changes (6)
- src/features/home/app/page-home.tsx
- src/lib/orpc/client.ts
- src/features/demo/demo-app-switch.tsx
- src/features/demo/demo-mode-drawer.tsx
- src/features/auth/config.ts
- src/features/dashboard/manager/page-dashboard.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/features/devtools/login-hint.tsx (1)
src/env/client.ts (1)
envClient(24-54)
🪛 Biome (2.1.2)
src/locales/ar/index.ts
[error] 10-10: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 10-10: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
src/locales/fr/index.ts
[error] 10-10: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 10-10: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
src/locales/en/index.ts
[error] 10-10: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 10-10: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
🪛 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). (4)
- GitHub Check: 🔬 Tests (lts/*)
- GitHub Check: 🔬 Tests (22)
- GitHub Check: 🔬 Tests (24)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (3)
src/locales/ar/index.ts (2)
10-10: Static analysis false positive on import assertion syntax.The Biome parser is flagging a missing semicolon, but the syntax is correct—the semicolon is present before the comment. This appears to be a parser limitation with import assertions rather than an actual issue. The code is valid and will execute correctly.
You may want to check if upgrading Biome or adjusting its configuration resolves this false positive, though it doesn't affect functionality.
10-10: Documentation comments added successfully.The !STARTERCONF comments clearly mark demo-related code for removal, which aligns well with the PR's goal of improving the onboarding experience.
Also applies to: 23-23
src/env/client.ts (1)
28-33: VITE_IS_DEMO implementation is type-safe and correct.The environment variable is properly configured with:
- Enum constraint for type safety ('true'/'false' only)
- Safe default value ('false')
- Correct boolean transformation
- Clear documentation comment for future removal
| import common from './common.json' assert { type: 'json' }; | ||
| import components from './components.json' assert { type: 'json' }; | ||
| import demo from './demo.json' assert { type: 'json' }; | ||
| import demo from './demo.json' assert { type: 'json' }; // !STARTERCONF [demoMode] Remove this import and the file |
There was a problem hiding this comment.
Move starter comment above the import/export to satisfy Biome.
Biome now fails with Expected a semicolon or an implicit semicolon after a statement on this line: the trailing // !STARTERCONF … placed after the assert { type: 'json' } import (and the matching export) breaks its parser. Shift the starter note onto its own line immediately before the statement so linting/CI can succeed.
- import demo from './demo.json' assert { type: 'json' }; // !STARTERCONF [demoMode] Remove this import and the file
+// !STARTERCONF [demoMode] Remove this import and the file
+import demo from './demo.json' assert { type: 'json' };
- demo, // !STARTERCONF [demoMode] Remove this export
+ // !STARTERCONF [demoMode] Remove this export
+ demo,Also applies to: 23-23
🧰 Tools
🪛 Biome (2.1.2)
[error] 10-10: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 10-10: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
🤖 Prompt for AI Agents
In src/locales/en/index.ts around lines 10 and 23, the trailing inline starter
comment after the import/export statement breaks Biome's parser; move the
starter comment onto its own line immediately above the affected import and
export statements (i.e., place "// !STARTERCONF [demoMode] Remove this import
and the file" on a separate line before the import at line 10 and the
corresponding export at line 23) so the import/export end with a proper
semicolon and linting/CI will succeed.
| import common from './common.json' assert { type: 'json' }; | ||
| import components from './components.json' assert { type: 'json' }; | ||
| import demo from './demo.json' assert { type: 'json' }; | ||
| import demo from './demo.json' assert { type: 'json' }; // !STARTERCONF [demoMode] Remove this import and the file |
There was a problem hiding this comment.
Same Biome parse error: relocate the inline starter comment.
Biome reports the identical parse failure here because the starter annotation trails the assert import/export. Move the comment onto the preceding line so linting resumes working.
- import demo from './demo.json' assert { type: 'json' }; // !STARTERCONF [demoMode] Remove this import and the file
+// !STARTERCONF [demoMode] Remove this import and the file
+import demo from './demo.json' assert { type: 'json' };
- demo, // !STARTERCONF [demoMode] Remove this export
+ // !STARTERCONF [demoMode] Remove this export
+ demo,Also applies to: 23-23
🧰 Tools
🪛 Biome (2.1.2)
[error] 10-10: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 10-10: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
🤖 Prompt for AI Agents
In src/locales/fr/index.ts around lines 10 and 23, the inline starter comment
sits after an import/assert expression causing Biome parse errors; relocate the
starter annotation to the previous line (place the comment immediately above the
import/export statement it annotates) for both occurrences so the comment does
not trail the import/assert syntax and linting/parsing will succeed.
Add comments for easy start with the starter
Summary by CodeRabbit
New Features
Bug Fixes
Chores