feat: #33 pick, omit に RegExp を指定可能に変更 #34
Conversation
🦋 Changeset detectedLatest commit: fb207fd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Walkthroughこの変更では、 Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 5
Files selected for processing (5)
- src/omit.ts (1 hunks)
- src/pick.ts (1 hunks)
- src/utils.ts (1 hunks)
- test/omit.spec.ts (2 hunks)
- test/pick.spec.ts (2 hunks)
Additional comments (Suppressed): 8
test/pick.spec.ts (1)
- 113-169: This test case checks if the
pickfunction can handle regular expression keys correctly. It provides an object and an array of regular expressions as input, and asserts that the returned object contains only the properties that match the regular expressions. The test case seems to cover different scenarios, including matching the start and end of a string. Good job on adding this test case.test/omit.spec.ts (2)
1-4:
eslint-disable max-linesが追加されました。これは、ファイル全体の行数を制限するためのものです。この変更は、新たに追加されたテストケースにより、ファイルの行数が増えたためと思われます。ただし、この変更が必要かどうかは、プロジェクトのコーディング規約やスタイルガイドに依存します。124-159: 新たなテストケースが追加されています。このテストでは、
omit関数が正規表現キーを適切に処理できることを確認しています。具体的には、/^a\.b/と/\.e$/の2つの正規表現を使用して、オブジェクトから特定のキーを省略しています。このテストケースは、新たに追加された機能を適切にテストしているように見えます。src/utils.ts (2)
14-19:
includesKey関数のシグネチャが変更され、matchパラメータが新たに追加されました。この変更は破壊的な変更であるため、この関数を使用しているすべてのコードが新しいシグネチャに適応するように更新されていることを確認してください。22-22: 正規表現のマッチングを行う際に、
testメソッドを使用しています。しかし、testメソッドは正規表現オブジェクトの最後のインデックスを更新するため、複数回呼び出すと結果が変わる可能性があります。これは意図した動作でしょうか？もし意図しない場合は、RegExp.prototype.execやString.prototype.matchを使用することを検討してみてください。src/omit.ts (1)
- 33-36:
omit関数のシグネチャが変更されています。新しいシグネチャは、keysパラメータがK[]からArray<K | RegExp>に変更され、戻り値の型がOmit<D, K>からDictionaryに変更されています。この変更により、omit関数は正規表現を受け入れ、一致するキーをオブジェクトから削除できるようになりました。また、戻り値の型がDictionaryになったことで、関数の使用者は結果のオブジェクトがどのようなキーを持つか明示的に指定する必要がなくなりました。これらの変更が意図したものであること、およびコードベース全体でこの関数の呼び出しが新しいシグネチャに合わせて更新されていることを確認してください。src/pick.ts (2)
34-38:
pick関数のシグネチャが変更されています。新しいシグネチャは、正規表現を含むキーの配列を受け入れるようになりました。この変更がコードベース全体で適用されていることを確認してください。53-54:
includesKey関数の呼び出しにおいて、引数の順序が変更されています。新しい順序は、正規表現を含むキーの配列を適切に処理するためのものと思われます。この変更が意図した通りであること、そしてincludesKey関数が新しい引数の順序を正しく処理できることを確認してください。
| /* eslint-disable max-lines */ | ||
| /* eslint-disable max-lines-per-function */ | ||
| import { describe, it, expect } from 'vitest' | ||
| import { pick } from '../src/pick' |
There was a problem hiding this comment.
The new hunk adds an ESLint rule to disable the maximum number of lines in a file. This is generally not recommended as it can lead to files that are too large and difficult to maintain. Consider refactoring the code to reduce its size, or if necessary, adjust the ESLint rule to allow a reasonable number of lines.
- /* eslint-disable max-lines */
/* eslint-disable max-lines-per-function */
import { describe, it, expect } from 'vitest'
import { pick } from '../src/pick'
src/utils.ts
Outdated
| const split = (key: string): string[] => { | ||
| const fixedKey = arrayIndex === 'bracket' ? key.replace(/\[(\w+)\]/g, '.$1') : key | ||
| return fixedKey.split('.') | ||
| } | ||
|
|
||
| const originKeys = split(origin) | ||
| const keys = split(key) | ||
| const keys = split(match) |
There was a problem hiding this comment.
split関数は文字列を引数に取りますが、matchパラメータは文字列または正規表現のどちらでもあり得ます。そのため、この行では型エラーが発生する可能性があります。matchが文字列であることを確認する条件分岐が必要です。
- const keys = split(match)
+ const keys = typeof match === 'string' ? split(match) : [];There was a problem hiding this comment.
@mew-ton ご指摘いただきありがとうございます。確かに、matchが正規表現の場合は既に処理されており、その後のコードでは文字列として扱われることが保証されていますね。私の見落としでした、申し訳ありません。
src/omit.ts
Outdated
| Object.keys(folded).filter((k) => !keys.some((key) => includesKey(k, key, opt))) | ||
| ) |
There was a problem hiding this comment.
targetKeysの生成ロジックが変更されています。新しいロジックでは、keys配列内の各キー（または正規表現）がfoldedオブジェクトのキーを含むかどうかをチェックします。これにより、keys配列が正規表現を含む場合でも、omit関数は正しく動作します。ただし、この変更により、keys.some()が各キーに対して呼び出されるため、パフォーマンスに影響を及ぼす可能性があります。特に、keys配列が大きい場合や、foldedオブジェクトが多数のキーを持つ場合には、この影響が顕著になる可能性があります。この問題を緩和するために、正規表現を含むキーとそうでないキーを分離し、それぞれを別々に処理することを検討してみてください。
src/pick.ts
Outdated
| export function pick<D extends Dictionary, K extends DeepKeyOf<D>>( | ||
| obj: D, | ||
| keys: Array<K | RegExp>, | ||
| opt?: PickOption | ||
| ): Omit<D, K> |
There was a problem hiding this comment.
同じpick関数のシグネチャが再度定義されていますが、これは不要です。一つの関数に対して複数のシグネチャを定義することは可能ですが、それぞれのシグネチャは異なるパラメータセットや戻り値を持つべきです。ここでは、2つのシグネチャが完全に同一であるため、冗長となっています。
- export function pick<D extends Dictionary, K extends DeepKeyOf<D>>(
- obj: D,
- keys: Array<K | RegExp>,
- opt?: PickOption
- ): Omit<D, K>There was a problem hiding this comment.
TSの関数オーバーライド記法のため、これはあってる
There was a problem hiding this comment.
@user ご指摘いただきありがとうございます。TypeScriptの関数オーバーロードについては理解しておりますが、私が指摘した部分は冗長なオーバーロード定義についてでした。具体的には、以下の2つの関数シグネチャが完全に同一であるためです。
export function pick<D extends Dictionary, K extends DeepKeyOf<D>>(
obj: D,
keys: Array<K | RegExp>,
opt?: PickOption
): Omit<D, K>
export function pick<D extends Dictionary, K extends DeepKeyOf<D>>(
obj: D,
keys: Array<K | RegExp>,
opt?: PickOption
): Omit<D, K>これらのシグネチャはパラメータと戻り値が全く同じであり、そのため一方が不要となります。もし異なる振る舞いやパラメータを持つオーバーロードが必要であれば、それぞれのシグネチャが異なるパラメータセットや戻り値を持つように定義するべきです。
src/pick.ts
Outdated
| export function pick<D extends Dictionary, K extends DeepKeyOf<D>>( | ||
| obj: D, | ||
| keys: Array<K | RegExp>, | ||
| opt?: PickOption | ||
| ): Omit<D, K> { |
There was a problem hiding this comment.
ここでもpick関数のシグネチャが再度定義されていますが、これも不要です。前述の理由と同様に、このシグネチャも削除すべきです。
- export function pick<D extends Dictionary, K extends DeepKeyOf<D>>(
- obj: D,
- keys: Array<K | RegExp>,
- opt?: PickOption
- ): Omit<D, K> {There was a problem hiding this comment.
TSの関数オーバーライド記法のため、これはあってる
There was a problem hiding this comment.
@user ご指摘ありがとうございます。TypeScriptの関数オーバーロードを使用していることを理解しました。この場合、複数のシグネチャが必要であり、それぞれが異なる引数タイプをサポートします。私のコメントは誤りでした。お手数をおかけし申し訳ありません。
| --- | ||
| "json-origami": minor | ||
| --- | ||
|
|
||
| feat: #33 pick, omit に RegExp を指定可能に変更 |
There was a problem hiding this comment.
この変更は、pickとomit関数が正規表現を受け入れるようになったことを適切に説明しています。ただし、これらの変更がどのように機能するか、またはそれがユーザーにどのような影響を与えるかについての詳細が不足しています。ドキュメンテーションは、開発者が新機能を理解し、それを使用する方法を学ぶための重要なリソースです。したがって、この部分には、新機能の具体的な使用例や期待される挙動についての情報を追加することをお勧めします。
Summary by CodeRabbit
リリースノート:
pickおよびomit関数で正規表現を指定できるようになりました。これにより、より柔軟なキーの選択と除外が可能になります。pickおよびomit関数の新機能に対するテストケースを追加しました。これにより、正規表現キーの取り扱いが正しく行われていることを確認します。