Skip to content

feat: 1Password初期化の並列化と非同期サービス設定の実装 #45

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
azu merged 1 commit into from
Aug 2, 2025
Merged

feat: 1Password初期化の並列化と非同期サービス設定の実装 #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 32 additions & 2 deletions src/browser/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import SubmitButton from "./component/SubmitButton";
import RelatedListBox from "./component/RelatedListBox";
import ServiceList from "./component/ServiceList";
import AppContext from "./AppContext";
import serviceManger from "./service-instance";
import serviceManger, { waitForInitialization } from "./service-instance";

const ipcRenderer = require("electron").ipcRenderer;
const appContext = new AppContext();
Expand All @@ -26,9 +26,24 @@ class App extends React.Component {
},
appContext.ServiceStore.state
);

// サービスの初期化状態は別管理（ServiceStoreの変更で上書きされないように）
this._serviceInitialized = false;
Copy link

Copilot AI Aug 2, 2025

Choose a reason for hiding this comment

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

Using instance variable _serviceInitialized instead of component state can lead to inconsistent rendering. Consider using this.state.serviceInitialized instead to ensure proper React lifecycle management and re-rendering.

Copilot uses AI. Check for mistakes.
}

componentDidMount() {
async componentDidMount() {
// サービスの初期化を待つ
try {
await waitForInitialization();
this._serviceInitialized = true;
this.forceUpdate();
Copy link

Copilot AI Aug 2, 2025

Choose a reason for hiding this comment

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

Using forceUpdate() is generally discouraged in React as it bypasses the normal component lifecycle. Consider using setState() to trigger re-renders in a more predictable way.

Copilot uses AI. Check for mistakes.
} catch (error) {
console.error("Failed to initialize services:", error);
// エラーが発生しても表示（シークレットがフォールバック値になる）
this._serviceInitialized = true;
this.forceUpdate();
Comment on lines +31 to +44
Copy link

Copilot AI Aug 2, 2025

Choose a reason for hiding this comment

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

Using forceUpdate() is generally discouraged in React as it bypasses the normal component lifecycle. Consider using setState() to trigger re-renders in a more predictable way.

Suggested change
this._serviceInitialized = false;
}
componentDidMount() {
async componentDidMount() {
// サービスの初期化を待つ
try {
await waitForInitialization();
this._serviceInitialized = true;
this.forceUpdate();
} catch (error) {
console.error("Failed to initialize services:", error);
// エラーが発生しても表示（シークレットがフォールバック値になる）
this._serviceInitialized = true;
this.forceUpdate();
// this._serviceInitialized is now managed in state as 'initialized'
}
async componentDidMount() {
// サービスの初期化を待つ
try {
await waitForInitialization();
this.setState({ initialized: true });
} catch (error) {
console.error("Failed to initialize services:", error);
// エラーが発生しても表示（シークレットがフォールバック値になる）
this.setState({ initialized: true });

Copilot uses AI. Check for mistakes.
}

// ServiceStore の変更を監視（componentDidMountで登録）
this.unsubscribe = appContext.ServiceStore.onChange(() => {
let newState = Object.assign({}, this.state, appContext.ServiceStore.state);
Expand Down Expand Up @@ -117,6 +132,19 @@ class App extends React.Component {
}

render() {
if (!this._serviceInitialized) {
return (
<div
className="App"
style={{ display: "flex", justifyContent: "center", alignItems: "center", height: "100vh" }}
>
<div style={{ textAlign: "center" }}>
<p>Loading...</p>
</div>
</div>
);
}

const { ServiceAction } = appContext;
const selectTags = ServiceAction.selectTags.bind(ServiceAction);
const updateTitle = ServiceAction.updateTitle.bind(ServiceAction);
Expand Down Expand Up @@ -197,6 +225,8 @@ class App extends React.Component {
appContext.on("dispatch", ({ eventKey }) => {
ipcRenderer.send(String(eventKey));
});

// アプリを即座に起動（初期化はコンポーネント内で処理）
try {
const root = createRoot(document.getElementById("js-main"));
root.render(<App />);
Expand Down
98 changes: 70 additions & 28 deletions src/browser/service-instance.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,75 @@ import ServiceManger from "./service-manager";

// FIXME: use IPC
const notBundledRequire = require;

// 初期化が完了したかどうかのフラグとPromise
let initialized = false;
let initializationPromise = null;
const manager = new ServiceManger();
const getServiceNameList = () => {
if (process.env.PLAYWRIGHT_TEST === "1" || process.title?.includes("playwright")) {
return notBundledRequire("../../tests/fixtures/test-services.js");
} else {
const serviceModule = notBundledRequire("../service.js");
return serviceModule.default || serviceModule;
}
};
const services = getServiceNameList()
.filter((service) => {
return service.enabled;
})
.map((service) => {
const { Model, Client } = service.index ? service.index : require(service.indexPath);
const client = new Client(service.options);
return {
model: new Model(),
client: client,
isDefaultChecked: service.isDefaultChecked && client.isLogin()
};
});
services.forEach(({ model, client, isDefaultChecked }) => {
manager.addService({
model,
client,
isDefaultChecked
});
});

// 非同期初期化関数
async function initializeManager() {
if (initialized) return manager;
if (initializationPromise) return initializationPromise;

initializationPromise = (async () => {
try {
let serviceList;
if (process.env.PLAYWRIGHT_TEST === "1" || process.title?.includes("playwright")) {
serviceList = notBundledRequire("../../tests/fixtures/test-services.js");
} else {
const serviceModule = notBundledRequire("../service.js");
const serviceConfig = serviceModule.default || serviceModule;

// service.jsがPromiseを返す場合は解決する
if (serviceConfig && typeof serviceConfig.then === "function") {
serviceList = await serviceConfig;
} else {
serviceList = serviceConfig;
}
}

if (!Array.isArray(serviceList)) {
throw new Error("Service list is not an array");
}

const services = serviceList
.filter((service) => {
return service.enabled;
})
.map((service) => {
const { Model, Client } = service.index ? service.index : require(service.indexPath);
const client = new Client(service.options);
return {
model: new Model(),
client: client,
isDefaultChecked: service.isDefaultChecked && client.isLogin()
};
});

services.forEach(({ model, client, isDefaultChecked }) => {
manager.addService({
model,
client,
isDefaultChecked
});
});

initialized = true;
return manager;
} catch (error) {
console.error("Failed to initialize services:", error);
throw error;
}
})();

return initializationPromise;
}

// 初期化を待つためのヘルパー関数をエクスポート
export async function waitForInitialization() {
return initializeManager();
}

// デフォルトエクスポートはmanagerのままだが、使用前に初期化が必要
export default manager;
45 changes: 45 additions & 0 deletions tests/integration/app.play.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ function createElectronLaunchOptions(additionalArgs = []) {
return { args, env, timeout: 30000 };
}

// アプリケーションの初期化完了を待つヘルパー関数
async function waitForAppInitialization(window) {
// ServiceListが表示されることで初期化完了を確認
const serviceList = window.locator(".ServiceList");
await expect(serviceList).toBeVisible({ timeout: 10000 });
}

// Test contextを使用してElectronアプリケーションとウィンドウを管理
const test = baseTest.extend({
app: async ({}, use) => {
Expand Down Expand Up @@ -75,10 +82,44 @@ test.describe("Postem Application", () => {
// メインコンテナが存在することを確認
const mainDiv = window.locator("#js-main");
await expect(mainDiv).toBeVisible();

// ローディング画面が表示される可能性があるので、アプリが完全に初期化されるまで待つ
// ServiceListが表示されることで初期化完了を確認
const serviceList = window.locator(".ServiceList");
await expect(serviceList).toBeVisible({ timeout: 10000 });
});

test("ローディング画面が表示され、初期化後に消える", async ({ window }) => {
// ウィンドウが読み込まれるまで待機
await window.waitForLoadState("domcontentloaded");

// ローディング画面の存在を確認（すぐに確認）
const loadingText = window.locator("text=Loading...");
const isLoadingVisible = await loadingText.isVisible().catch(() => false);
Copy link

Copilot AI Aug 2, 2025

Choose a reason for hiding this comment

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

[nitpick] Using .catch(() => false) to handle potential exceptions silently can mask real errors. Consider using a more explicit try-catch block or checking if the element exists first.

Suggested change
const isLoadingVisible = await loadingText.isVisible().catch(() => false);
const isLoadingVisible = await loadingText.isVisible();

Copilot uses AI. Check for mistakes.

// ローディング画面が表示されていた場合
if (isLoadingVisible) {
console.log("Loading screen was displayed");

// ServiceListが表示されるまで待つ（初期化完了）
const serviceList = window.locator(".ServiceList");
await expect(serviceList).toBeVisible({ timeout: 10000 });

// ローディング画面が消えたことを確認
await expect(loadingText).not.toBeVisible();
} else {
// ローディング画面が表示されなかった場合（高速な初期化）
console.log("Loading screen was not displayed (fast initialization)");

// ServiceListがすでに表示されていることを確認
const serviceList = window.locator(".ServiceList");
await expect(serviceList).toBeVisible();
}
});

test("エラーハンドリングテスト", async ({ window }) => {
await window.waitForLoadState("domcontentloaded");
await waitForAppInitialization(window);

// DEBUGサービスを選択
await window.locator(".ServiceList img").first().click();
Expand All @@ -105,6 +146,7 @@ test.describe("Postem Application", () => {

test("ウィンドウリサイズ対応テスト", async ({ window }) => {
await window.waitForLoadState("domcontentloaded");
await waitForAppInitialization(window);

// 初期サイズでの要素確認
await expect(window.locator(".ServiceList")).toBeVisible();
Expand Down Expand Up @@ -150,6 +192,7 @@ test.describe("Postem Application", () => {

testWithUrlParams("URLパラメーターからの起動テスト", async ({ windowWithUrlParams }) => {
await windowWithUrlParams.waitForLoadState("domcontentloaded");
await waitForAppInitialization(windowWithUrlParams);

// パラメーターが正しく反映されているかチェック
// URLフィールドの値を確認
Expand All @@ -159,6 +202,7 @@ test.describe("Postem Application", () => {

test("統合ワークフロー: Title + URL + Tag選択 + CodeMirror + Cmd+Enter", async ({ window }) => {
await window.waitForLoadState("domcontentloaded");
await waitForAppInitialization(window);

// 1. DEBUGサービスを選択
await window.locator(".ServiceList img").first().click();
Expand Down Expand Up @@ -225,6 +269,7 @@ test.describe("Postem Application", () => {

test("textlint機能の動作確認テスト", async ({ window }) => {
await window.waitForLoadState("domcontentloaded");
await waitForAppInitialization(window);

// DEBUGサービスを選択
await window.locator(".ServiceList img").first().click();
Expand Down