Skip to content

backmerge #148

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.

Sign up for GitHub

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

Merged
KinjiKawaguchi merged 5 commits into from
Apr 7, 2026
Merged

backmerge #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
24eb2a3
next (#145)
KinjiKawaguchi Apr 4, 2026
3590973
3.0.0-rc.4
KinjiKawaguchi Apr 4, 2026
63b9fbd
3.0.0
KinjiKawaguchi Apr 4, 2026
2c0af6d
fix: Supabase Transaction pool mode 対応のため DB ドライバを postgres-js に切り替え …
KinjiKawaguchi Apr 5, 2026
12103bb
3.0.1
KinjiKawaguchi Apr 5, 2026
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
174 changes: 14 additions & 160 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@shizuoka-its/core",
"version": "3.0.0-rc.3",
"version": "3.0.1",
"description": "ITS core library",
"keywords": [],
"license": "ISC",
Expand Down Expand Up @@ -53,12 +53,11 @@
},
"dependencies": {
"drizzle-orm": "^0.45.1",
"pg": "^8.20.0",
"postgres": "^3.4.8",
"uuid": "^13.0.0"
},
"devDependencies": {
"@types/node": "^25.5.0",
"@types/pg": "^8.20.0",
"drizzle-kit": "^0.31.10",
"typescript": "^6.0.2",
"vite-plus": "^0.1.13"
Expand Down
18 changes: 10 additions & 8 deletions src/infrastructure/drizzle/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
import { drizzle } from "drizzle-orm/node-postgres";
import { Pool } from "pg";
/// <reference types="node" />
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

The /// <reference types="node" /> directive is the only occurrence in the repo and is likely redundant given @types/node is already a devDependency and tsconfig.json doesn’t restrict types. Keeping this can cause the generated .d.ts for this module to include an implicit dependency on Node types for downstream consumers. Prefer removing the directive (or, if Node types truly need to be forced project-wide, configure it via tsconfig.json instead).

Suggested change
/// <reference types="node" />

Copilot uses AI. Check for mistakes.
import { drizzle } from "drizzle-orm/postgres-js";
import postgres, { type Sql } from "postgres";
import * as schema from "./schema";

let pool: Pool | null = null;
let client: Sql | null = null;

function getPool(): Pool {
if (!pool) {
function getClient(): Sql {
if (!client) {
const connectionString = process.env.DATABASE_URL;
if (!connectionString) {
throw new Error("DATABASE_URL environment variable is not set");
}
pool = new Pool({ connectionString });
// Supabase の Transaction pool mode は prepared statement をサポートしないため無効化
client = postgres(connectionString, { prepare: false });
Comment on lines +14 to +15
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot Apr 7, 2026

Choose a reason for hiding this comment

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

📝 Info: prepare: false disables prepared statements globally

Setting prepare: false at src/infrastructure/drizzle/client.ts:15 is necessary for Supabase Transaction pool mode (as noted in the comment), but it disables prepared statements for all queries. This has a minor performance implication — the database server will parse and plan every query from scratch rather than reusing cached plans. This is a reasonable trade-off for Supabase compatibility, but worth noting if this library is also used against non-pooled PostgreSQL instances where prepared statements would be beneficial.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}
return pool;
return client;
}
Comment on lines +6 to 18
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot Apr 7, 2026

Choose a reason for hiding this comment

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

📝 Info: No connection cleanup / graceful shutdown handling

The postgres client exposes a .end() method for graceful shutdown, but neither the old pg.Pool code nor the new postgres code includes any cleanup mechanism. For a library package (this is @shizuoka-its/core), this is likely intentional — the consuming application is responsible for process lifecycle management. However, in long-running server environments, orphaned connections could be an issue if the module is loaded/unloaded dynamically.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


export function getDb() {
return drizzle(getPool(), { schema });
return drizzle(getClient(), { schema });
}
Comment on lines 20 to 22
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot Apr 7, 2026

Choose a reason for hiding this comment

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

📝 Info: getDb() creates a new drizzle instance on every call

Each call to getDb() at src/infrastructure/drizzle/client.ts:20-22 creates a new drizzle(getClient(), { schema }) wrapper, while the underlying postgres connection pool (client) is correctly cached as a singleton. This was the same pattern used with the previous pg Pool implementation. The drizzle wrapper is lightweight, so this is not a performance concern, but caching the drizzle instance (similar to how client is cached) would be a minor improvement for consistency.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


export type DrizzleDb = ReturnType<typeof getDb>;