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

refactor(nodecli): リファクタリング、文章の修正 #919

Merged
merged 9 commits into from
Aug 5, 2019

Conversation

azu
Copy link
Collaborator

@azu azu commented Aug 3, 2019

  • ファイル名を明示するように修正
  • process.exit(error.code) -> process.exit(1)
  • 文章の統一

fix #880

@@ -7,8 +7,10 @@ const filePath = program.args[0];
fs.readFile(filePath, "utf8", (err, file) => {
if (err) {
console.error(err);
process.exit(err.code);
// 終了ステータス 1(一般的なエラー)としてプロセスを終了
process.exit(1);
Copy link
Collaborator Author

@azu azu Aug 3, 2019

Choose a reason for hiding this comment

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

❯ node src/read-file-3.js  test
{ [Error: ENOENT: no such file or directory, open 'test'] errno: -2, code: 'ENOENT', syscall: 'open', path: 'test' }

~/.ghq/github.com/asciidwango/js-primer/source/use-case/nodecli/read-file 2019-nodecli-refactoring*
❯ echo $?
0

err.code はundefined なので 明示的に 1 を指定

@bot-user
Copy link

bot-user commented Aug 3, 2019

Deploy preview for js-primer ready!

Built with commit 62fd648

https://deploy-preview-919--js-primer.netlify.com

@azu
Copy link
Collaborator Author

azu commented Aug 3, 2019

@lacolaco nodecli どういう風に読んで書いていく構造になっている?
readあたりはファイル名が毎回違うけど、どんどんファイルを増やしながら実装していくイメージ?

でも https://jsprimer.net/use-case/nodecli/md-to-html/

前のセクションの最後で書いたスクリプトに、markedパッケージの読み込み処理を追加します。

とかを見ると同じファイルに追加してる感じになって、写しながらやっていくと結構困った。

また、ブラウザで実行するアプリケーションであっても、その開発にはツールとしてのNode.jsが欠かせません。
このセクションではユースケースの学習へ進むために必要なアプリケーション開発環境の準備をおこないます。

## Node.jsのインストール {#install-nodejs}

[Node.js][]はサーバーサイドJavaScript実行環境のひとつで、次のような特徴があります。

- WebブラウザのChromeと同じ[V8][] JavaScriptエンジンで動作する
- ウェブブラウザのChromeと同じ[V8][] JavaScriptエンジンで動作する
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ウェブブラウザに統一


ここでは `ajaxapp` という名前で新しいディレクトリを作成します。ここからは作成した`ajaxapp`ディレクトリ以下で作業していきます。

またこのプロジェクトで作成するファイルは、必ず文字コード(エンコーディング)を**UTF-8**、改行コードを**LF**にしてファイルを保存します。
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ユースケースに文字コードと改行コードの指定を追加


`--foo`オプションに値を与えて実行すれば、文字列が`options.foo`プロパティにセットされていることがわかります。
`--bar`オプションに値を与えて実行すれば、文字列が`options.bar`プロパティにセットされていることがわかります。
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

--foo だと被るので --bar に変更してみた


ECMAScriptで定義されているグローバルオブジェクトはブラウザとNode.jsどちらの環境にも存在します。
たとえば`Boolean``String`などのラッパーオブジェクトや、`Map`、`Array`、`Promise`のようなビルトインオブジェクトがそれにあたります
たとえば`Boolean``String`などのラッパーオブジェクト、`Map`や`Promise`のようなビルトインオブジェクトはどちらの環境にも存在します
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Arrayはややこしいので、省いた

今回のアプリケーションでは次の2つのオプションを扱います。

- gfm
- sanitize
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

marked #918 sanitizeオプションの問題出てきそう。
バージョンは固定されてるけど将来消える

テストコード中でエラーを投げるために、今回はNode.jsの標準モジュールのひとつである[assertモジュール][]から提供される`assert`関数を利用します
`assert`関数は引数の評価結果がfalseであるとき、実行時にエラーを投げます
テストコード中でエラーを投げるために、今回はNode.jsの標準モジュールのひとつである[assertモジュール][]から提供される`assert.strictEqual`メソッドを利用します
`assert.strictEqual`メソッドは第1引数と第2引数の評価結果が`===`で比較して異なる場合に、例外を投げる関数です
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

エラーのときの情報量を考えた assert.strictEqual に変更した。


```json
{
...
"scripts": {
"test": "mocha"
"test": "mocha test/"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

なんか ディレクトリが暗黙的に決まるのは説明的にもしにくいので test/ を明示するようにした。

@@ -107,7 +110,15 @@ Mochaのユニットテストは`test`ディレクトリの中にJavaScriptフ
`test/fixtures`ディレクトリにはユニットテストで用いるファイルを配置しています。
今回は変換元のMarkdownファイルと、期待する変換結果のHTMLファイルの2つが存在します。

ユニットテストを記述したら、もう一度改めて`npm test`コマンドを実行しましょう。1件のテストが通れば成功です。
次のように変換元のMarkdownファイルを`test/fixtures/sample.md`に配置します。
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

実際にどういうファイルを作るのかをちゃんと書くようにした。

@@ -118,6 +129,12 @@ $ npm test
1 passing (18ms)
```

ユニットテストが通らなかった場合は、次のことを確認してみましょう
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://pagain.hatenablog.com/entry/2019/06/24/120820 テスト通らなかったときの確認事項を適当に足してみた。

module.exports.foo = function() { /**/ };
module.exports.bar = function() { /**/ };
module.exports.foo = function() {
console.log("foo関数が呼び出されました");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

実際に書いて試してる人は何かよばれたのかを確認したいと思うのでログを出す

@azu azu requested a review from lacolaco August 3, 2019 13:52
@azu azu marked this pull request as ready for review August 3, 2019 14:06
@@ -1,5 +1,5 @@
const program = require("commander");
program.option("--foo <text>");
program.option("--bar <text>");
Copy link
Collaborator Author

@azu azu Aug 3, 2019

Choose a reason for hiding this comment

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

<text> ってどういう意味なのか結局よくわかってない
type checkとかあるのかな

@lacolaco
Copy link
Collaborator

lacolaco commented Aug 4, 2019

@lacolaco nodecli どういう風に読んで書いていく構造になっている?
readあたりはファイル名が毎回違うけど、どんどんファイルを増やしながら実装していくイメージ?

でも https://jsprimer.net/use-case/nodecli/md-to-html/

前のセクションの最後で書いたスクリプトに、markedパッケージの読み込み処理を追加します。

とかを見ると同じファイルに追加してる感じになって、写しながらやっていくと結構困った。

commander-param.jsとか分けてるけど、多分main.jsでやっていったほうがいいですよねえこれ

@lacolaco
Copy link
Collaborator

lacolaco commented Aug 4, 2019

なんか学び用のサンプルコードと、アプリのためのコードの区別が難しいんだな

@azu azu merged commit c23ab2e into master Aug 5, 2019
@azu
Copy link
Collaborator Author

azu commented Aug 5, 2019

とりあえずマージしました。

@lacolaco 残りは #928 にしました。

@azu azu deleted the 2019-nodecli-refactoring branch August 5, 2019 13:49
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.

npm: どのディレクトリでインストールするかを明示する
3 participants