🐛 Fix JSR publish #69
Conversation
WalkthroughThe changes focus on enhancing type checking functionalities in the codebase. New functions are added for various type checks like Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| jsr-publish: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: denoland/setup-deno@v1 | ||
| with: | ||
| deno-version: ${{ env.DENO_VERSION }} | ||
| - name: Publish (dry-run) | ||
| run: | | ||
| deno publish --dry-run |
There was a problem hiding this comment.
Consider adding a condition to only run the jsr-publish job on specific branches or under certain conditions.
jsr-publish:
+ if: github.ref == 'refs/heads/main'
runs-on: ubuntu-latestThis ensures that the publish dry-run only occurs in scenarios where it's relevant, such as merges to the main branch, preventing unnecessary runs on feature branches or in response to other events that wouldn't affect publishing.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| jsr-publish: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - uses: denoland/setup-deno@v1 | |
| with: | |
| deno-version: ${{ env.DENO_VERSION }} | |
| - name: Publish (dry-run) | |
| run: | | |
| deno publish --dry-run | |
| jsr-publish: | |
| if: github.ref == 'refs/heads/main' | |
| runs-on: ubuntu-latest | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - uses: denoland/setup-deno@v1 | |
| with: | |
| deno-version: ${{ env.DENO_VERSION }} | |
| - name: Publish (dry-run) | |
| run: | | |
| deno publish --dry-run |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #69 +/- ##
==========================================
- Coverage 99.40% 99.37% -0.03%
==========================================
Files 9 5 -4
Lines 669 641 -28
Branches 72 72
==========================================
- Hits 665 637 -28
Misses 4 4 ☔ View full report in Codecov by Sentry. |
Owing to the principle outlined below, defining everything within the `is.ts` file is considerably more straightforward. https://jsr.io/docs/about-slow-types#types-must-be-simply-inferred-or-explicit
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (4)
__snapshots__/is_test.ts.snapis excluded by!**/*.snapis/__snapshots__/_deprecated_test.ts.snapis excluded by!**/*.snapis/__snapshots__/annotation_test.ts.snapis excluded by!**/*.snapis/__snapshots__/utility_test.ts.snapis excluded by!**/*.snap
Files selected for processing (4)
- is.ts (1 hunks)
- is_bench.ts (1 hunks)
- is_test.ts (3 hunks)
- mod.ts (1 hunks)
Files not summarized due to errors (1)
- is.ts: Error: Message exceeds token limit
Files skipped from review due to trivial changes (2)
- is_bench.ts
- mod.ts
Additional comments not posted (34)
is.ts (30)
11-18: Ensure the usage ofSetfor primitive type checks is the most efficient approach for your use case.While using a
Setfor checking if a type is a primitive is correct, consider if direct comparison (e.g.,typeof x === "string") might be more performant or readable in some cases, especially if the set of types is small and unlikely to change.
61-63: The implementation ofisAnycorrectly always returnstrue, as expected for a type predicate that matches any type.
78-80: TheisUnknownfunction is implemented correctly, always returningtrueto indicate that any type is consideredunknown.
95-97: TheisStringfunction correctly checks if the input type is a string.
112-114: TheisNumberfunction is implemented correctly to check for number types.
129-131: TheisBigIntfunction accurately checks for bigint types.
146-148: TheisBooleanfunction correctly identifies boolean types.
163-167: TheisArrayfunction usesArray.isArraycorrectly to check for array types.
182-184: TheisSetfunction correctly checks for instances ofSet.
207-211: TheisRecordObjectfunction correctly checks for plain object instances.
234-237: TheisRecordfunction is implemented correctly to check for object types, including instances likeSet,Map, etc.
246-249: TheisRecordLikefunction is correctly marked as deprecated and its implementation matches the intended functionality.
265-267: TheisMapfunction correctly checks for instances ofMap.
282-284: TheisFunctionfunction correctly checks for function instances.
299-302: TheisSyncFunctionfunction correctly identifies synchronous functions usingObject.prototype.toString.
318-321: TheisAsyncFunctionfunction accurately identifies asynchronous functions.
337-339: TheisNullfunction correctly checks fornullvalues.
354-356: TheisUndefinedfunction is implemented correctly to check forundefinedvalues.
371-373: TheisNullishfunction correctly checks fornullorundefinedvalues.
388-390: TheisSymbolfunction correctly checks for symbol types.
414-415: TheisPrimitivefunction correctly checks for primitive types using a predefined set.
421-427: TheisOptionalfunction correctly checks if a predicate function is annotated asOptional.
445-464: TheisOptionalOffunction correctly creates anOptionalannotated type predicate function, with proper handling of existing optional predicates.
506-512: TheisReadonlyfunction correctly checks if a predicate function is annotated asReadonly.
534-544: TheisReadonlyOffunction correctly annotates a predicate function asReadonly, as intended.
754-828: TheisReadonlyTupleOffunction is correctly marked as deprecated and suggests usingis.ReadonlyOf(is.TupleOf(...))instead.
892-929: TheisReadonlyUniformTupleOffunction is correctly marked as deprecated, with a suggestion to useis.ReadonlyOf(is.UniformTupleOf(...))instead.
1583-1586: TheisOneOffunction is correctly marked as deprecated, with a suggestion to useisUnionOfinstead.
1600-1603: TheisAllOffunction is correctly marked as deprecated, with a suggestion to useisIntersectionOfinstead.
1617-1666: The exportedisobject correctly aggregates all the type predicates and utility functions, making them easily accessible.is_test.ts (4)
7-63: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-9]
The import statements are correctly structured and use pinned versions for Deno's standard library, which is a good practice.
107-495: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [110-1768]
The tests are well-structured and organized, using
Deno.testandawait t.stepfor clarity and readability.
107-495: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [110-1768]
Consider adding more test cases for complex type checking functions to ensure comprehensive coverage, especially for functions that involve nested or composite types.
107-495: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [110-1768]
Consider refactoring repetitive test patterns to reduce code duplication and enhance error messages for failed assertions to provide more context for debugging.
There was a problem hiding this comment.
Actionable comments posted: 2
Actionable comments outside the diff hunks (3)
metadata.ts (3)
Line range hint
17-21: Consider adding a comment explaining the rationale behind usingdeno-lint-ignore no-explicit-any.
Line range hint
29-44: Ensure robust error handling aroundObject.definePropertiesto gracefully handle potential failures.
Line range hint
46-50: LGTM! ThegetPredicateFactoryMetadatafunction is straightforward and effectively utilizesgetMetadata.
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (4)
__snapshots__/is_test.ts.snapis excluded by!**/*.snapis/__snapshots__/_deprecated_test.ts.snapis excluded by!**/*.snapis/__snapshots__/annotation_test.ts.snapis excluded by!**/*.snapis/__snapshots__/utility_test.ts.snapis excluded by!**/*.snap
Files selected for processing (5)
- is.ts (1 hunks)
- is_bench.ts (1 hunks)
- is_test.ts (3 hunks)
- metadata.ts (1 hunks)
- mod.ts (1 hunks)
Files not summarized due to errors (1)
- is.ts: Error: Message exceeds token limit
Files skipped from review as they are similar to previous changes (2)
- is_bench.ts
- mod.ts
Additional comments not posted (9)
metadata.ts (2)
1-1: Ensure thatPredicateis correctly exported from./is.tsto avoid import errors.Verification successful
The
Predicatetype is correctly exported fromis.ts, as verified by the script output. This confirms that the import statement inmetadata.tsis accurate and should not cause any import errors related toPredicate.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that Predicate is exported from is.ts rg "export type Predicate" src/is.tsLength of output: 88
Script:
#!/bin/bash # Search for the is.ts file across the entire repository fd is.tsLength of output: 16
Script:
#!/bin/bash # Verify that Predicate is exported from is.ts at the root of the repository rg "export type Predicate" is.tsLength of output: 158
1-4: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [4-10]
LGTM! The type definitions for handling metadata are well-designed and flexible.
is.ts (3)
60-63: Ensure the use ofanyis absolutely necessary and documented.// deno-lint-ignore no-explicit-any export function isAny(_x: unknown): _x is any { return true; }
1246-1253: Use specific types instead ofanywhere possible to enhance type safety.// deno-lint-ignore no-explicit-any export function isInstanceOf<T extends new (...args: any) => unknown>( ctor: T, ): Predicate<InstanceType<T>> & WithMetadata<IsInstanceOfMetadata> { return setPredicateFactoryMetadata( (x: unknown): x is InstanceType<T> => x instanceof ctor, { name: "isInstanceOf", args: [ctor] }, ); }
1617-1666: The export objectisis well-organized and provides a clear API for consumers. However, consider documenting each exported function for better developer experience.is_test.ts (4)
7-63: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-9]
Consider grouping imports from the same module to improve readability.
- import { - assertEquals, - assertStrictEquals, - } from "https://deno.land/std@0.211.0/assert/mod.ts"; - import { - assertSnapshot, - } from "https://deno.land/std@0.211.0/testing/snapshot.ts"; - import { assertType } from "https://deno.land/std@0.211.0/testing/types.ts"; + import { assertEquals, assertStrictEquals, assertSnapshot, assertType } from "https://deno.land/std@0.211.0/testing/types.ts";
10-60: Ensure that all imported functions are utilized within the file to avoid unnecessary imports.Verification successful
The verification process indicates that the imported functions from "./is.ts" are utilized within
is_test.ts. This suggests that the imports are necessary and correctly used in the file.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all imported functions from "./is.ts" are used in `is_test.ts`. rg --type ts 'is\w+' is_test.ts | cut -d: -f3 | sort | uniq # Expected to match the list of imported functions.Length of output: 17612
110-428: The structure and naming of the tests are consistent and follow best practices for Deno testing. However, consider adding more descriptive test names or comments to clarify the specific behavior being tested, especially for complex predicates.
1770-1772: Ensure that the dynamic import and filtering logic correctly identifies and tests all relevant functions from the./is.tsmodule. Consider adding explicit tests for any functions that might be missed by this dynamic approach.
| import type { FlatType, UnionToIntersection, Writable } from "./_typeutil.ts"; | ||
| import { | ||
| type GetMetadata, | ||
| getMetadata, | ||
| getPredicateFactoryMetadata, | ||
| type PredicateFactoryMetadata, | ||
| setPredicateFactoryMetadata, | ||
| type WithMetadata, | ||
| } from "./metadata.ts"; |
There was a problem hiding this comment.
Consider organizing imports to improve readability.
import type { FlatType, UnionToIntersection, Writable } from "./_typeutil.ts";
import {
type GetMetadata,
getMetadata,
getPredicateFactoryMetadata,
type PredicateFactoryMetadata,
setPredicateFactoryMetadata,
type WithMetadata,
} from "./metadata.ts";Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| import type { FlatType, UnionToIntersection, Writable } from "./_typeutil.ts"; | |
| import { | |
| type GetMetadata, | |
| getMetadata, | |
| getPredicateFactoryMetadata, | |
| type PredicateFactoryMetadata, | |
| setPredicateFactoryMetadata, | |
| type WithMetadata, | |
| } from "./metadata.ts"; |
| pred: Predicate<T>, | ||
| ): | ||
| & Predicate<T | undefined> | ||
| & WithMetadata<IsOptionalOfMetadata> { | ||
| if (isOptional(pred)) { | ||
| return pred as | ||
| & Predicate<T | undefined> | ||
| & WithMetadata<IsOptionalOfMetadata>; | ||
| } | ||
| return Object.defineProperties( | ||
| setPredicateFactoryMetadata( | ||
| (x: unknown): x is Predicate<T | undefined> => x === undefined || pred(x), | ||
| { name: "isOptionalOf", args: [pred] }, | ||
| ), | ||
| { optional: { value: true as const } }, | ||
| ) as | ||
| & Predicate<T | undefined> | ||
| & WithMetadata<IsOptionalOfMetadata>; | ||
| } | ||
|
|
There was a problem hiding this comment.
The implementation of isOptionalOf could be simplified by directly returning the modified predicate without the conditional check, as the metadata addition is idempotent.
export function isOptionalOf<T>(
pred: Predicate<T>,
):
& Predicate<T | undefined>
& WithMetadata<IsOptionalOfMetadata> {
- if (isOptional(pred)) {
- return pred as
- & Predicate<T | undefined>
- & WithMetadata<IsOptionalOfMetadata>;
- }
return Object.defineProperties(
setPredicateFactoryMetadata(
(x: unknown): x is Predicate<T | undefined> => x === undefined || pred(x),
{ name: "isOptionalOf", args: [pred] },
),
{ optional: { value: true as const } },
) as
& Predicate<T | undefined>
& WithMetadata<IsOptionalOfMetadata>;
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| export function isOptionalOf<T>( | |
| pred: Predicate<T>, | |
| ): | |
| & Predicate<T | undefined> | |
| & WithMetadata<IsOptionalOfMetadata> { | |
| if (isOptional(pred)) { | |
| return pred as | |
| & Predicate<T | undefined> | |
| & WithMetadata<IsOptionalOfMetadata>; | |
| } | |
| return Object.defineProperties( | |
| setPredicateFactoryMetadata( | |
| (x: unknown): x is Predicate<T | undefined> => x === undefined || pred(x), | |
| { name: "isOptionalOf", args: [pred] }, | |
| ), | |
| { optional: { value: true as const } }, | |
| ) as | |
| & Predicate<T | undefined> | |
| & WithMetadata<IsOptionalOfMetadata>; | |
| } | |
| export function isOptionalOf<T>( | |
| pred: Predicate<T>, | |
| ): | |
| & Predicate<T | undefined> | |
| & WithMetadata<IsOptionalOfMetadata> { | |
| return Object.defineProperties( | |
| setPredicateFactoryMetadata( | |
| (x: unknown): x is Predicate<T | undefined> => x === undefined || pred(x), | |
| { name: "isOptionalOf", args: [pred] }, | |
| ), | |
| { optional: { value: true as const } }, | |
| ) as | |
| & Predicate<T | undefined> | |
| & WithMetadata<IsOptionalOfMetadata>; | |
| } |
Summary by CodeRabbit
isAny,isUnknown,isSymbol,isPrimitive, etc.isfunction from theis.tsmodule.Predicatein themetadata.tsfile.isfrommod.ts.