Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ use_repo(node, "nodejs_toolchains")
npm = use_extension("@aspect_rules_js//npm:extensions.bzl", "npm")
npm.npm_translate_lock(
name = "npm_rulesjs",
bins = {
# vite's bin is not auto-detected due to peer-dep version qualifiers;
# declare it explicitly so vite_binary() is generated.
"vite": ["vite=./bin/vite.js"],
},
pnpm_lock = "//:pnpm-lock.yaml",
)
use_repo(npm, "npm_rulesjs")
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ GOOGLE_CLOUD_PROJECT=septima-dev \
FIREBASE_AUTH_EMULATOR_HOST=localhost:9099 \
bazel run //backend:dev_server

# Terminal 3 — Vite-powered React frontend (not yet implemented)
# Terminal 3 — Vite-powered React frontend (http://localhost:5173)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description still mentions "3 Node native test runner tests," but the implementation (and the previous audit) correctly identifies 5 tests in frontend/index.test.ts. Please update the summary to reflect the excellent additional coverage.

bazel run //frontend:dev_server
```

> `//frontend:dev_server` is not yet implemented. Until it lands, only the backend and test targets above are available.
> Copy `frontend/.env.local.example` to `frontend/.env.local` and fill in the Firebase config values before running the frontend dev server.

### Updating dependencies

Expand Down
58 changes: 55 additions & 3 deletions frontend/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,29 +1,81 @@
load("@aspect_rules_js//js:defs.bzl", "js_test")
load("@aspect_rules_ts//ts:defs.bzl", "ts_project")
load("@npm_rulesjs//:vite/package_json.bzl", vite_bin = "bin")

ts_project(
name = "frontend",
srcs = glob(
["*.ts"],
exclude = ["*.test.ts"],
exclude = [
"*.test.ts",
"vite.config.ts",
],
),
declaration = True,
transpiler = "tsc",
tsconfig = "tsconfig.json",
visibility = ["//visibility:public"],
Comment on lines 4 to 17
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted in previous feedback, the frontend target (and the accompanying frontend/index.ts) appears to be a redundant shell now that the source logic has moved into src/ and frontend_lib. We should remove this target and the index.ts file to keep the Bazel build graph clean and avoid developer confusion.

)

ts_project(
name = "frontend_lib",
srcs = glob(
[
"src/**/*.ts",
"src/**/*.tsx",
],
exclude = ["src/**/*.test.*"],
),
declaration = True,
transpiler = "tsc",
tsconfig = "tsconfig.json",
visibility = ["//visibility:public"],
deps = [
"//:node_modules/@types/react",
"//:node_modules/@types/react-dom",
"//:node_modules/firebase",
"//:node_modules/react",
"//:node_modules/react-dom",
"//:node_modules/vite",
],
)

ts_project(
name = "tests_lib",
srcs = ["index.test.ts"],
declaration = True,
transpiler = "tsc",
tsconfig = "tsconfig.json",
deps = ["//:node_modules/@types/node"],
deps = [
":frontend_lib",
"//:node_modules/@types/node",
],
)

js_test(
name = "test",
data = [":tests_lib"],
data = [
":frontend_lib",
":tests_lib",
],
entry_point = "index.test.js",
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dev_server target includes glob(["src/**"]) in its data. Since frontend_lib also depends on these files, this is slightly redundant, though it ensures Vite has access to the raw .tsx files for transformation. It's acceptable as-is but worth a note for future refactors of the frontend build rules.

vite_bin.vite_binary(
name = "dev_server",
args = [
"--port=5173",
"--host",
],
chdir = package_name(),
data = [
"index.html",
"tsconfig.json",
"vite.config.ts",
":frontend_lib",
"//:node_modules/@vitejs/plugin-react",
"//:node_modules/firebase",
"//:node_modules/react",
"//:node_modules/react-dom",
] + glob(["src/**"]),
)
12 changes: 12 additions & 0 deletions frontend/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Septima</title>
</head>
<body>
<div id="root"></div>
<script type="module" src="/src/main.tsx"></script>
</body>
</html>
95 changes: 92 additions & 3 deletions frontend/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,95 @@
import test from 'node:test';
import assert from 'node:assert';
import test from 'node:test';
import { createApiClient } from './src/apiClient.js';

test('attaches Authorization Bearer header', async () => {
let capturedInit: RequestInit | undefined;
const mockFetch = async (
_url: string | URL | Request,
init?: RequestInit,
): Promise<Response> => {
capturedInit = init;
return new Response(JSON.stringify({}), {
status: 200,
headers: { 'Content-Type': 'application/json' },
});
};

const client = createApiClient(
async () => 'test-token',
'http://localhost:8080',
mockFetch as typeof fetch,
);
await client.post('/ping');

const headers = capturedInit?.headers as Record<string, string>;
assert.strictEqual(headers['Authorization'], 'Bearer test-token');
});

test('sends POST to the correct URL', async () => {
let capturedUrl: string | URL | Request = '';
const mockFetch = async (
url: string | URL | Request,
_init?: RequestInit,
): Promise<Response> => {
capturedUrl = url;
return new Response(JSON.stringify({}), {
status: 200,
headers: { 'Content-Type': 'application/json' },
});
};

const client = createApiClient(
async () => 'token',
'http://localhost:8080',
mockFetch as typeof fetch,
);
await client.post('/ping');
assert.strictEqual(capturedUrl, 'http://localhost:8080/ping');
});

test('throws on non-ok response with status code', async () => {
const mockFetch = async (): Promise<Response> =>
new Response('', { status: 401 });
const client = createApiClient(
async () => 'token',
'http://localhost:8080',
mockFetch as typeof fetch,
);
await assert.rejects(
() => client.post('/ping'),
(err: Error) => {
assert.match(err.message, /HTTP 401/);
return true;
},
);
});

test('includes server error body in thrown error', async () => {
const mockFetch = async (): Promise<Response> =>
new Response('Unauthorized: token expired', { status: 401 });
const client = createApiClient(
async () => 'token',
'http://localhost:8080',
mockFetch as typeof fetch,
);
await assert.rejects(
() => client.post('/ping'),
(err: Error) => {
assert.match(err.message, /token expired/);
return true;
},
);
});

test('Simple Test', () => {
assert.strictEqual(true, true);
test('returns undefined for 204 No Content', async () => {
const mockFetch = async (): Promise<Response> =>
new Response(null, { status: 204 });
const client = createApiClient(
async () => 'token',
'http://localhost:8080',
mockFetch as typeof fetch,
);
const result = await client.post('/trigger');
assert.strictEqual(result, undefined);
});
3 changes: 3 additions & 0 deletions frontend/src/App.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function App() {
return <div>Septima</div>;
}
27 changes: 27 additions & 0 deletions frontend/src/apiClient.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
export function createApiClient(
getToken: () => Promise<string>,
baseUrl: string,
fetcher: typeof fetch = fetch,
) {
return {
async post<T>(path: string, body?: unknown): Promise<T> {
const token = await getToken();
const response = await fetcher(new URL(path, baseUrl).toString(), {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new URL(path, baseUrl) constructor has a well-known pitfall regarding leading and trailing slashes. If baseUrl is http://api/v1 (no trailing slash), v1 is dropped. If path is /ping (leading slash), the entire path of baseUrl is replaced by the root. Given our reliance on environment variables for baseUrl, this is highly likely to cause silent misconfigurations. We should either sanitize the inputs (e.g., baseUrl.replace(//$/, '') + '/' + path.replace(/^//, '')) or use a robust joining utility.

method: 'POST',
headers: {
Authorization: `Bearer ${token}`,
'Content-Type': 'application/json',
},
body: body != null ? JSON.stringify(body) : undefined,
});
if (!response.ok) {
const detail = await response.text().catch(() => '');
throw new Error(
`HTTP ${response.status}${detail ? `: ${detail}` : ''}`,
Comment on lines +18 to +20
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the error path, response.text() is used to provide detail. If the server (or a proxy) returns a large HTML error page instead of a short string, the thrown Error message could become unwieldy. Consider truncating the detail string to a reasonable length (e.g., 200 characters) before including it in the error message.

);
}
const text = await response.text();
return (text ? JSON.parse(text) : undefined) as T;
},
};
}
32 changes: 32 additions & 0 deletions frontend/src/firebase.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { initializeApp } from 'firebase/app';
import { connectAuthEmulator, getAuth } from 'firebase/auth';

function requireEnv(key: string, value: string | undefined): string {
if (!value)
throw new Error(
`Missing required env var: ${key}. Copy frontend/.env.local.example to frontend/.env.local and fill in the values.`,
);
return value;
}

const firebaseConfig = {
apiKey: requireEnv(
'VITE_FIREBASE_API_KEY',
import.meta.env.VITE_FIREBASE_API_KEY,
),
authDomain: requireEnv(
'VITE_FIREBASE_AUTH_DOMAIN',
import.meta.env.VITE_FIREBASE_AUTH_DOMAIN,
),
projectId: requireEnv(
'VITE_FIREBASE_PROJECT_ID',
import.meta.env.VITE_FIREBASE_PROJECT_ID,
),
};

export const app = initializeApp(firebaseConfig);
export const auth = getAuth(app);

if (import.meta.env.DEV) {
connectAuthEmulator(auth, 'http://localhost:9099', { disableWarnings: true });
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Firebase Auth emulator URL remains hardcoded to http://localhost:9099. While this is the default, our README.md already suggests using FIREBASE_AUTH_EMULATOR_HOST. We should make this configurable via a VITE_ environment variable to ensure developers in containerized or custom environments can connect their frontend to the local services.

}
9 changes: 9 additions & 0 deletions frontend/src/main.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { StrictMode } from 'react';
import { createRoot } from 'react-dom/client';
import { App } from './App';

createRoot(document.getElementById('root')!).render(
<StrictMode>
<App />
</StrictMode>,
);
1 change: 1 addition & 0 deletions frontend/src/vite-env.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/// <reference types="vite/client" />
2 changes: 2 additions & 0 deletions frontend/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
{
"compilerOptions": {
"target": "ESNext",
"lib": ["ESNext", "DOM", "DOM.Iterable"],
"module": "ESNext",
"moduleResolution": "node",
"jsx": "react-jsx",
"strict": true,
"esModuleInterop": true,
"skipLibCheck": true,
Expand Down
9 changes: 9 additions & 0 deletions frontend/vite.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import react from '@vitejs/plugin-react';
import { defineConfig } from 'vite';

export default defineConfig({
plugins: [react()],
build: {
sourcemap: true,
},
});
Loading