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

サンプルコードのJSONが絶対パスとURLの2パターン存在する #273

Closed
harry0000 opened this issue Aug 15, 2016 · 3 comments · Fixed by #274
Closed

サンプルコードのJSONが絶対パスとURLの2パターン存在する #273

harry0000 opened this issue Aug 15, 2016 · 3 comments · Fixed by #274

Comments

@harry0000
Copy link

サンプルコードを書き写しながら勉強していた際に気になった点をフィードバックします。

詳細

サンプルコードでcomment.json等が使用されていますが、絶対パスとURLの2パターンが混在しています。
絶対パスで記載されているのはCh2_HowToWrite\promise-resolve.adocだけのようです。

Ch2_HowToWrite\promise-resolve.adoc#L68

var promise = Promise.resolve($.ajax('/json/comment.json'));// => promiseオブジェクト

Ch2_HowToWrite\src\multiple-xhr-callback-counter.js#L32

        return getURLCallback('http://azu.github.io/promises-book/json/comment.json', parse.bind(null, callback));

問題点

絶対パス('/json/comment.json') で記載されている方が先に登場する(P.16)。

  • 絶対パスの例は、そのまま書き写しても動かない
  • 読み進めないと、このJSONのURLが分からない

ただ、この絶対パスをURLにしてしまうとコードが長くなって改行が発生し、結果的に読みづらくなりそうです。
注釈かなにかでURLが補足されていると良さそうな気がしました、個人的に。

@azu
Copy link
Owner

azu commented Aug 15, 2016

$.ajax('/json/comment.json');// => .then をもつオブジェクト

こちらの方はそもそもjQuery.ajaxはthenを持つオブジェクトを返すというだけの例なので、実際に実行させる意図はないですね。(コンソールのボタンもだしてない)

なので、jQuery.ajaxでなくてもいいのですが、jQueryで見慣れてる非同期はajaxかなと思って使ってた感じですね

@azu
Copy link
Owner

azu commented Aug 15, 2016

後者の方もCORSの問題があるため、azu.github.io のドメイン上で実行しないと動かないという問題がありますね。。
(なので大体の例はCORS対応している httpbin を使っているのですが、httpbinだとレスポンスの種類が少ない…)

なので、前者はあくまでAPIの例であって実行させる意図ではないこと
後者はCORSの問題について注記するのが良さそうな気がしています。

@harry0000
Copy link
Author

コメント頂きありがとうございます。意図について理解できました。
そのような対応でよいかと思います。

おそらく私がpdf版で読んでいたために起きた問題な気がしています。
(リポジトリもcloneしておらず、JSONの存在に気が付かなかったため)

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 a pull request may close this issue.

2 participants