Add 'occupational-basilisk-4371' to Featured Vibes #280
Conversation
✅ Deploy Preview for fireproof-ai-builder ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
- Overwriting
window.locationdirectly in tests risks cross-test pollution and missing Location properties; mock the getter and restore after each test instead. [tests/vibe-route.test.tsx] - Tests relying on
Date.nowandsetTimeout(10)are flaky; switch to fake timers and fixed system time. [tests/useSimpleChat/saveCode.test.tsx] vite.server.allowedHostswas narrowed to a single Netlify host, likely breaking ngrok and other dev flows; make it configurable and include prior defaults. [vite.config.ts]- Minor: misleading comment in Vite config still references ngrok despite setting a Netlify domain.
Summary of changes
- Updated multiple tests to reflect eager Fireproof initialization and removed legacy API key/credit checks.
- Rewrote useSession tests to mock
use-fireproofdirectly and assert eager DB init calls; adjusted expectations accordingly. - Removed post-login effect test and added new saveCodeAsAiMessage tests for
useSimpleChat. - Modified
vibe-routetests to assert redirect viawindow.location.replaceinstead of iframe embedding. - Added a settings-route detection test to
useViewState. - Tweaked AuthContext mocks across tests to align with current hook usage.
- Updated tsconfig (exclude stories, module interop options) and Vite config (changed
server.allowedHosts).
tests/vibe-route.test.tsx
Outdated
| // Mock window.location.replace to prevent navigation errors | ||
| const mockReplace = vi.fn(); | ||
| Object.defineProperty(window, 'location', { | ||
| value: { | ||
| replace: mockReplace, | ||
| search: '', | ||
| }, | ||
| writable: true, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Overwriting window.location with a plain object can leak into other tests and break code that expects a full Location object (e.g., origin, href). It’s also not restored after tests, risking cross-test pollution. Prefer mocking the getter for window.location to override just replace and restore it in afterEach.
Suggestion
Use a getter spy and restore after each test instead of redefining window.location:
const mockReplace = vi.fn();
const originalLocation = window.location;
beforeEach(() => {
vi.spyOn(window, 'location', 'get').mockReturnValue({
...originalLocation,
replace: mockReplace as any,
search: '',
} as any);
mockReplace.mockClear();
});
afterEach(() => {
vi.restoreAllMocks();
});Then remove the Object.defineProperty(window, 'location', ...) block. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.
| it('ensures user edit messages always have the newest timestamp', async () => { | ||
| const wrapper = createWrapper(); | ||
| const { result } = renderHook(() => useSimpleChat('test-session'), { wrapper }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(result.current.saveCodeAsAiMessage).toBeDefined(); | ||
| }); | ||
|
|
||
| // First edit - should create new messages | ||
| await result.current.saveCodeAsAiMessage('console.log("first edit");', result.current.docs); | ||
| const afterFirstEdit = Date.now(); | ||
|
|
||
| // Wait a bit to ensure different timestamps | ||
| await new Promise((resolve) => setTimeout(resolve, 10)); | ||
|
|
||
| // Second edit - should create new messages with newer timestamps | ||
| const beforeSecondEdit = Date.now(); | ||
| await result.current.saveCodeAsAiMessage('console.log("second edit");', result.current.docs); | ||
| const afterSecondEdit = Date.now(); | ||
|
|
||
| // This test verifies the function doesn't throw and can be called multiple times | ||
| // The actual timestamp verification would need more complex mocking to test properly | ||
| // The real test is in the implementation behavior where timestamps are always current | ||
| expect(beforeSecondEdit).toBeGreaterThanOrEqual(afterFirstEdit); | ||
| expect(afterSecondEdit).toBeGreaterThanOrEqual(beforeSecondEdit); | ||
| }); |
There was a problem hiding this comment.
The Date.now + setTimeout(10) pattern introduces timing flakiness and can intermittently fail on slow CI. These tests don’t need real time—use fake timers and fixed system time to make them deterministic.
Suggestion
Adopt fake timers and fixed system time in this suite:
import { vi } from 'vitest';
beforeEach(() => {
vi.useFakeTimers();
vi.setSystemTime(new Date('2024-01-01T00:00:00Z'));
});
afterEach(() => {
vi.useRealTimers();
});Then replace await new Promise((r) => setTimeout(r, 10)); with await vi.advanceTimersByTimeAsync(10);. This removes flakiness while preserving the intent. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.
vite.config.ts
Outdated
| server: { | ||
| host: '0.0.0.0', // Listen on all local IPs | ||
| allowedHosts: ['.ngrok-free.app'], // Specific ngrok hostname | ||
| allowedHosts: ['devserver-main--fireproof-ai-builder.netlify.app'], // Specific ngrok hostname |
There was a problem hiding this comment.
Changing server.allowedHosts from a permissive pattern (.ngrok-free.app) to a single Netlify hostname will block dev flows that rely on ngrok or other hostnames. The inline comment also references ngrok while the value is a Netlify domain, which is misleading. Consider supporting both (and potentially more) via an environment variable.
Suggestion
Parameterize allowed hosts and keep sensible defaults for local tooling:
const defaultAllowedHosts = ['.ngrok-free.app', 'devserver-main--fireproof-ai-builder.netlify.app'];
const allowedHostsEnv = process.env.VITE_ALLOWED_HOSTS
? process.env.VITE_ALLOWED_HOSTS.split(',').map((s) => s.trim()).filter(Boolean)
: undefined;
server: {
host: '0.0.0.0',
allowedHosts: allowedHostsEnv ?? defaultAllowedHosts,
cors: true,
hmr: true,
watch: { ignored: ['**/tests/**'] },
}Also update the comment to accurately reflect the Netlify hostname. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.
|
rebase to main and add https://vivaldi-quasar-3156_pingpong.vibesdiy.app/ also call it Bricksalot -- @CharlieHelps |
|
rebase to main and add https://vivaldi-quasar-3156_pingpong.vibesdiy.app/ also call it Bricksalot -- @CharlieHelps |
Expand this to see my work.
|
There was a problem hiding this comment.
- tests/vibe-route.test.tsx replaces
window.locationwholesale, risking cross-test pollution and breaking APIs; mock the getter and restore instead. - useSimpleChat saveCode tests rely on real time (
Date.now,setTimeout), causing flakiness; switch to fake timers and advance explicitly. - Vite
server.allowedHostsnarrowed to a single Netlify host, likely breaking ngrok and other dev flows; parameterize and keep broader defaults. The inline comment is also misleading. - Other test refactors look aligned with recent architectural changes (eager Fireproof init, removed credits), with no apparent issues.
Additional notes (2)
-
Maintainability |
tests/useSession.test.ts:1-5
vi.mock('use-fireproof', ...)is declared after importing bothuseSessionanduseFireproof. While Vitest hoistsvi.mock, relying on that transform is brittle—importing modules that internally importuse-fireproofbefore the mock can lead to the real module being used in some bundler setups. Make the mock definition unambiguously run before those imports. -
Maintainability |
tests/useSimpleChat/setup.tsx:11-12
callCountin thecall-aimock accumulates across tests and isn’t reset, creating inter‑test coupling. If any test order changes or additional tests are added, expectations may inadvertently depend on prior calls.
Summary of changes
- Updated tests to align with eager Fireproof initialization (useSession) and removed obsolete API key/credit provisioning mocks.
- Added a new test suite for useSimpleChat.saveCodeAsAiMessage and enhanced useViewState tests to detect the settings view from URL path.
- Adjusted test wrappers to avoid unnecessary AuthProvider use; standardized env mocks (including CALLAI_ENDPOINT) for hermetic tests.
- Modified vite.config.ts to change server.allowedHosts to a specific Netlify domain; updated tsconfig.json to exclude Storybook files and tweak compiler options.
- Simplified various test setups and removed a deprecated post-login effect test.
See files: tests/useSession.test.ts, tests/useSimpleChat/saveCode.test.tsx, tests/useSimpleChat-codeReady.test.ts, tests/vibe-route.test.tsx, vite.config.ts, tsconfig.json.
…d 'Create Emotional Playlists' (occupational-basilisk-4371) to featured list\n\n- Rebased branch to latest main by resetting to origin/main and reapplying minimal change\n- Per request in PR #280 (2025-10-13): add https://vivaldi-quasar-3156_pingpong.vibesdiy.app/ and call it Bricksalot
charliecreates
bot
force-pushed
the
ai-274-add-to-featured-vibes
branch
from
a9eca6e to
e8052bf
Compare
|
Rebased this branch to the latest main and added the requested vibe.
Commit: e8052bf Changes
Verification# Local CI bundle
$ pnpm check
# Results (summarized)
- Build: OK
- Tests: 136 files, 657 passed, 86 skipped, 0 failed
- Lint: OK
- Format: applied
# URL checks
$ curl -sSIL https://vivaldi-quasar-3156_pingpong.vibesdiy.app/ | head -n1
HTTP/2 200
$ curl -sSIL https://vivaldi-quasar-3156_pingpong.vibesdiy.app/screenshot.png | head -n1
HTTP/2 200
$ curl -sSIL https://occupational-basilisk-4371.vibesdiy.app/ | head -n1
HTTP/2 200
$ curl -sSIL https://occupational-basilisk-4371.vibesdiy.app/screenshot.png | head -n1
HTTP/2 200Notes:
@jchris this should match your request: rebased to main, added the new URL, and labeled it “Bricksalot.” |
Adds the vibe identified by the slug
occupational-basilisk-4371to the Featured Vibes collection.Context
app/components/FeaturedVibes.tsx(publishedVibesarray)<title>tag: “Create Emotional Playlists”.Changes
{ name: 'Create Emotional Playlists', slug: 'occupational-basilisk-4371' }at the top ofpublishedVibes.Impact
count).publishedVibesand slices bycount, so adding an item simply increases the candidate pool.Verification
Closes #274