Skip hot-reloading if build fails #480
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
omochi
commented
Jun 8, 2024
| if !buildResult.succeeded { | ||
| Diagnostics.remark("[Plugin] **Build Failed**") | ||
| print(buildResult.logText) | ||
| print(buildResult.logText, terminator: "") |
There was a problem hiding this comment.
ログが末尾改行を含んでいるので print の改行を除きます。
| if !buildResult.succeeded { | ||
| Diagnostics.remark("[Plugin] **Build Failed**") | ||
| print(buildResult.logText) | ||
| print(buildResult.logText, terminator: "") | ||
| responseMessage = Data([0]) |
| } else { | ||
| Diagnostics.remark("[Plugin] **Build Succeeded**") | ||
| responseMessage = Data([1]) |
| case .failedToOpenBuildRequestPipe: "failed to open build request pipe" | ||
| case .failedToOpenBuildResponsePipe: "failed to open build response pipe" | ||
| } | ||
| enum DevCommandError: Error & CustomStringConvertible { |
There was a problem hiding this comment.
CartonFrontendDevCommand.Error だと型の外部から使うとき長いので、
トップレベルに移動して名前を簡潔にしました。
| case noBuildResponseOption | ||
| case failedToOpenBuildRequestPipe | ||
| case failedToOpenBuildResponsePipe | ||
| case pluginConnectionClosed |
| case failedToOpenBuildRequestPipe | ||
| case failedToOpenBuildResponsePipe | ||
| case pluginConnectionClosed | ||
| case brokenPluginResponse |
There was a problem hiding this comment.
プラグインから来たメッセージが壊れていた場合を追加
| @@ -202,6 +206,20 @@ struct SwiftPMPluginBuilder: BuilderProtocol { | |||
| func run() async throws { | |||
| // We expect single response per request | |||
| try buildRequest.write(contentsOf: Data([1])) | |||
| _ = try buildResponse.read(upToCount: 1) | |||
| guard let responseMessage = try buildResponse.read(upToCount: 1) else { | |||
There was a problem hiding this comment.
readの結果に応じて様々な例外を投げ分けます。
| @@ -63,6 +63,10 @@ extension Event: Decodable { | |||
| } | |||
| } | |||
|
|
|||
| public struct BuilderProtocolSimpleBuildFailedError: Error { | |||
There was a problem hiding this comment.
現状、プラグイン側でコンパイラのエラーを出力していて、フロントエンド側では何も出す必要がありません。そのような、ただビルドができなかったことだけを伝えるためのエラー型を専用に定義します。
ビルドに失敗するのはある意味正常な通信の結果であり、
プラグインとの通信に失敗するなど、実際のビルド以前のレベルで失敗する場合もありえます。
これはエラー内容を出力します。
| default: | ||
| terminal.write("\(error)\n", inColor: .red) | ||
| } | ||
| return |
There was a problem hiding this comment.
ビルドエラーの通知を受けたらその旨を表示します。
シンプルなビルドの失敗ではなかった場合は、その例外の情報も表示します。
そして脱出します。
|
|
||
| terminal.write("\nBuild completed successfully\n", inColor: .green, bold: false) | ||
| terminal.write("Build completed successfully\n", inColor: .green) |
There was a problem hiding this comment.
成功した場合はリロードさせます。
先頭の改行を消します。
デフォルトの non bold は引数指定が余計なので消します。
kateinoigakukun
approved these changes
Jun 8, 2024
kateinoigakukun
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
課題
devコマンドはファイルの変更があるとリビルドをしますが、
この時、変更によってソースコードが壊れた場合、コンパイルに失敗することがあります。
しかし、プラグインはそれをフロントエンドに伝えていないため、
コンパイルに失敗したにも関わらず、
Build completed successfullyと出力してからブラウザをリロードさせます。ユーザがコードの編集中にこれが生じた場合、
successの表示と共にブラウザがリロードされたので、
ユーザは編集結果がブラウザにホットリロードされたと期待するでしょう。
しかし実際にはバイナリが更新されていないため、
動作確認がうまくいかず混乱するでしょう。
コンパイルエラーが出ていることは、
ターミナルの少し上を見なければわからず、
気づかない事があるでしょう。
提案
プラグインからフロントエンドにビルドが成功したかどうかを伝え、
失敗した場合にはそれを表示して、ブラウザのリロードはスキップします。
詳細
以下に変更前後の出力の変化を掲載します。
余計な空行も削除しています。
変更前, リビルド成功時
変更前, リビルド失敗時
変更前, リビルド成功時
変更前, リビルド失敗時