unimal-jp / spearly-sdk-js

Apache License 2.0
0 stars 1 forks source link

コンストラクタの修正提案 #56

Closed mantaroh closed 1 year ago

mantaroh commented 1 year ago

Astro + Spearly CMS を利用してみて、数行で連携することができました!

実際に使ってみて、以下の点が気になったのでコンストラクタを変えたいと思いますが、いかがでしょうか?

コンストラクタ

現在の仕様

const apiClient = new SpearlyApiClient(SPEARLY_DOMAIN, API_KEY)

気になる点

変更提案

SpearliApiClient の引数を以下のように指定する

  1. APIキー・ドメインではなく、API認証ヘッダ・API URL を指定するようにする
const apiClient = new SpearlyApiClient(SPEARLY_API_URL, API_AUTHORIZE_HEADER)
  1. ドメインを オプション指定とする
const apiClient = new SpearlyApiClient(API_KEY)

※この場合は、API_KEY を設定からコピペしてくださいというやり方で誘導できるので、わかりやすいかもしれません。

akoarum commented 1 year ago

@mantaroh SPEARLY_DOMAINSPEARLY_API_URL (optional) は良さそうだなと思いますが、APIキーだけ渡せばすぐ使えるようにしたい意図があるので、API_KEYはヘッダにしたくないなと思っています。

そしてそもそもとして、改めて見ていると埋め込み方法のAPIキーに Authorization: Bearer が含まれてしまっていることがそもそものミスリードな気がするので、埋め込み方法のページの記述を変更しつつ、「2. ドメインを オプション指定とする」の対応を取るのが良さそうかなと感じています。

mantaroh commented 1 year ago

@akoarum

ありがとうございますー!

改めて見ていると埋め込み方法のAPIキーに Authorization: Bearer が含まれてしまっていることがそもそものミスリードな気がする

そうですよね。こちらは、公式ドキュメントを修正してもらえるか問い合わせてみます!

「2. ドメインを オプション指定とする」の対応を取るのが良さそうかなと感じています。

こちらはオプションとして指定するように修正してみようと思います 🙇🏼‍♂️