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

feat: createClient関数で生成される関数の引数にreadonlyを追加 #60

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

totto2727
Copy link

@totto2727 totto2727 commented Jul 9, 2023

背景

MicroCMSQueryfieldsにリテラル型のタプルを渡した時、以下のような型エラーが発生します。

// 型エラーの発生する最小構成
const client = createClient({ "apiKey": '', serviceDomain: "" })
client.get({ endpoint: '', queries: { fields: ['id'] as const } })
Type 'readonly ["id"]' is not assignable to type 'string | string[] | undefined'.
  The type 'readonly ["id"]' is 'readonly' and cannot be assigned to the mutable type 'string[]'.ts(2322)

解決策

  • createClient関数で生成される関数の引数の全てにreadonlyを追加しました
    • 再帰的にreadonlyを追加するためts-essentialsを導入しました
    • 引数に対して破壊的変更を行う処理が存在しないため、readonlyを追加しても問題ないと判断しました

懸念点

  • typeof client['get']のように型を再利用しながら、引数に破壊的な変更を行う既存コードが壊れる可能性があります

    const client = createClient({ "apiKey": '', serviceDomain: "" })
    const get: typeof client['get'] = (p) => {
      if(typeof p.queries?.fields === 'object'){
        p.queries.fields.push() // ここで以下の型エラー
      }
    }
    Property 'push' does not exist on type 'readonly string[]'.ts(2339)
    

備考

  • makeRequest関数でfetchClient関数にrequestInitを渡す際、readonlyのなしのRequestInitにキャストしています
    • generateFetchClient関数を確認した限り、キャストした部分の破壊的変更は存在しなかったため、このような実装にしています
  • フォークしたリポジトリを使用すると背景の型エラーは解決されることを確認しています

@totto2727
Copy link
Author

関連する可能性のあるIssue
#18

関連する個人開発の実装
https://github.com/totto2727/microcms-astro/blob/6e3af5e5b169c9d6df0392ad7af634120a8427d2/src/config/microCMS.ts#L35C9-L35C9

@himorishige himorishige self-requested a review August 24, 2023 06:56
@himorishige
Copy link
Contributor

@totto2727
ご対応ありがとうございます!
また確認が遅くなり申し訳ありません🙏
実装上は問題なく採用させていただきたいと思っています。

microCMSサイトのドキュメントやすでに実装されている方への影響も考えられるためマージとリリースのタイミングについては検討のお時間をもうしばらくいただけると幸いです🙇‍♂️

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.

None yet

2 participants