Skip to content
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.

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

errorToMessage関数追加し、createLoggerの処理を変更して、eslintのルールを1つ採用 #2212

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

Hiroshiba
Copy link
Member

内容

こちらのコメントでやり取りしていた内容です。

errorToMessage関数を作って、あとロガーをちょっと改造してtoString()メソッドがなくても大丈夫なようにしました。
これによってeslintで例外にしていたルールを1つ採用できます。

ちなみにテンプレート文字列に突っ込む時、String関数でラップするのとしないのとでは若干挙動が違うらしい。
https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/String#%E6%96%87%E5%AD%97%E5%88%97%E3%83%97%E3%83%AA%E3%83%9F%E3%83%86%E3%82%A3%E3%83%96%E3%81%A8_string_%E3%82%AA%E3%83%96%E3%82%B8%E3%82%A7%E3%82%AF%E3%83%88
String関数を使うとSymbolもそれっぽい文字列にしてくれるらしいけど、テンプレート文字列(.toString()?)だとエラーになるらしい。

関連 Issue

スクリーンショット・動画など

その他

@Hiroshiba Hiroshiba requested a review from a team as a code owner August 11, 2024 16:40
@Hiroshiba Hiroshiba force-pushed the errorToMessage作る branch from a2b10ad to 5d42154 Compare August 11, 2024 16:41
Copy link
Member Author

Choose a reason for hiding this comment

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

ファイル名が変わりました

throw new Error(`Key not found: ${key}`);
throw new Error(`Key not found: ${String(key)}`);
Copy link
Member Author

Choose a reason for hiding this comment

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

元のコードだとSymbolが来たときにエラーになってた

@Hiroshiba Hiroshiba changed the title errorToMessage関数追加し、createLoggerの引数の型を変更して、eslintのルールを1つ採用 errorToMessage関数追加し、createLoggerの処理を変更して、eslintのルールを1つ採用 Aug 11, 2024
Comment on lines 4 to 9
const createInner =
(method: "logInfo" | "logError" | "logWarn") =>
(...args: unknown[]) => {
window.backend[method](`[${scope}] ${args[0]}`, ...args.slice(1));
window.backend[method](`[${scope}]`, ...args);
};
return {
Copy link
Member Author

Choose a reason for hiding this comment

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

ここの処理変更によって、console.logだとなんかいい感じに表示される"%o", 25のような書式が使えなくなってしまいます。
たぶん現コードのどこにも使われてないけれども、この変更していいのかが判断しづらい。。
(テンプレート文字列の方が使いやすい気はする)

Copy link
Member

@sevenc-nanashi sevenc-nanashi left a comment

Choose a reason for hiding this comment

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

大丈夫だと思います。

@Hiroshiba
Copy link
Member Author

レビューありがとうございます!マージします!!

@Hiroshiba Hiroshiba merged commit 535ca9c into VOICEVOX:main Aug 12, 2024
9 checks passed
@Hiroshiba Hiroshiba deleted the errorToMessage作る branch August 12, 2024 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants