-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Chore(deps): Bump packages #134
Conversation
|
WalkthroughThe changes in this pull request involve updates to various Changes
Possibly related PRs
Suggested labels
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
error Couldn't find any versions for "eslint-config-custom" that matches "workspace:^" 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Triggered from #134 by @black7375. Checking if we can fast forward Target branch ( commit d0991f539e651abf074aea91ca2dcd3078a0cd08 (HEAD -> main, origin/main, origin/HEAD)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Nov 23 22:51:35 2024 +0900
Fix: Rules - Prop expression type limitation #131 Pull request ( commit 842689aca2986f939ab0436c44976c01567a4544 (pull_request/bump)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sun Dec 1 19:56:38 2024 +0900
Chore(deps): Bump packages It is possible to fast forward |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (16)
packages/transform-to-vanilla/src/transform.ts (1)
53-54
: Repeated use of@ts-ignore
forimport.meta
Similar to the previous instance, these
@ts-ignore
comments suppress TypeScript errors forimport.meta
. Adjusting the TypeScriptmodule
setting could remove the need for these suppressions.packages/transform-to-vanilla/src/transform-values/simply-important.ts (1)
11-13
: Consider updating TypeScript configuration instead of using @ts-ignore.While the @ts-ignore comments work, a better approach would be to update the TypeScript configuration to support ES modules properly. This would eliminate the need for these comments across multiple files.
Consider adding the following to your
tsconfig.json
:{ "compilerOptions": { "module": "node16", "moduleResolution": "node16" } }Also applies to: 15-16
packages/transform-to-vanilla/src/transform-keys/complex-selectors.ts (1)
23-25
: Consider adding @ts-expect-error instead of @ts-ignoreWhile the TS ignores are necessary for CommonJS compatibility, using
@ts-expect-error
would be more explicit about the expected error.-// @ts-ignore error TS1343 +// @ts-expect-error TS1343: The 'import.meta' meta-property is only allowed when the '--module' option is 'es2020', 'es2022', 'esnext', 'system', 'node16', or 'nodenext'.Also applies to: 27-28
packages/transform-to-vanilla/src/types/string.ts (1)
Line range hint
35-59
: LGTM! Well-structured type testsThe test suite effectively validates type transformations with:
- CamelCase to KebabCase
- PascalCase to KebabCase
- Kebab to CamelCase/PascalCase
- Colon to snake case conversions
Consider adding more edge cases:
// Additional test cases to consider: type EmptyString = ToKebabCase<"">; type SingleChar = ToKebabCase<"A">; type MultipleUpperCase = ToKebabCase<"XMLHttpRequest">;packages/transform-to-vanilla/src/transform-keys/merge-key.ts (1)
18-23
: Consider using a shared utility for import.meta handlingThe TypeScript ignore comments for import.meta are being repeated across multiple files. Consider extracting this into a shared utility file to maintain DRY principles.
Create a shared test utility file that handles the import.meta setup:
// test-utils.ts export const getTestUtils = () => { // @ts-ignore error TS1343 if (import.meta.vitest) { // @ts-ignore error TS1343 return import.meta.vitest; } return null; };packages/transform-to-vanilla/src/transform-values/css-var.ts (1)
119-124
: LGTM! Consistent module compatibility handling.The added TypeScript ignore comments align with the codebase-wide approach to handling import.meta in CommonJS environments.
This consistent pattern of handling import.meta across multiple files suggests we could consider:
- Moving to ESM modules in the future to avoid these TypeScript ignores
- Creating a shared test utility that handles this CommonJS compatibility layer
packages/transform-to-vanilla/src/types/utils.ts (1)
Line range hint
74-143
: Comprehensive type testing coverageThe test suite thoroughly validates all utility types with good edge cases. Consider adding a few more test cases:
IntRange
with negative numbersPartialDeepMerge
with arraysSpread
with more than two objectspackages/transform-to-vanilla/src/transform-keys/at-rules.ts (1)
Line range hint
78-171
: Well-structured test suite with comprehensive coverageThe test suite effectively covers:
- At-rule detection
- Nested rules
- Anonymous rules
- Key merging strategies
Consider adding tests for:
- Error cases (invalid at-rules)
- Complex nested scenarios
packages/css/src/css/index.ts (2)
Line range hint
73-98
: Consider performance optimization for variant processingThe
processVariants
function usesstructuredClone
which can be performance-intensive for large objects. Consider implementing the suggested lazy getter pattern mentioned in the TODO comment.Example optimization:
- const context = structuredClone(initTransformContext); + const context = { ...initTransformContext };
Line range hint
166-166
: Address the TODO comment for variant reference testingThe comment indicates missing tests for variant references with globalCSS(). This is important for ensuring the reliability of variant processing.
Would you like me to help implement the missing test cases for variant references?
packages/transform-to-vanilla/src/transform-object/rule-context.ts (1)
131-133
: Consider using a more maintainable approach for handling CommonJS compatibility.Instead of using multiple individual
@ts-ignore
comments, consider using a single@ts-expect-error
with a more descriptive comment at the module level.-// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore error TS1343: The 'import.meta' meta-property is only allowed when the '--module' option is 'es2020', 'es2022', 'esnext', 'system', 'node16', or 'nodenext'. +// @ts-expect-error Module compilation target doesn't support import.metapackages/transform-to-vanilla/src/types/style-rule.ts (2)
257-259
: Maintain consistency in error handling across files.Apply the same error handling approach suggested for rule-context.ts here.
-// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore error TS1343: The 'import.meta' meta-property is only allowed when the '--module' option is 'es2020', 'es2022', 'esnext', 'system', 'node16', or 'nodenext'. +// @ts-expect-error Module compilation target doesn't support import.meta
Line range hint
260-577
: Comprehensive type testing with well-structured test cases.The test suite effectively validates CSS rule types and their combinations. Consider these improvements:
- Add JSDoc comments explaining expected failure conditions in negative test cases
- Consider adding test cases for complex CSS variable combinations
Example of improved test documentation:
/** * Tests invalid variant selection to ensure type safety. * Expected to fail when: * 1. Using undefined variant values * 2. Using variant values not in the original definition */ it("Invalid VariantSelection Type", () => { // existing test });packages/css/src/rules/types.ts (1)
172-177
: Maintain consistency in error handling across the codebase.Apply the same error handling approach suggested for other files.
-// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore error TS1343: The 'import.meta' meta-property is only allowed when the '--module' option is 'es2020', 'es2022', 'esnext', 'system', 'node16', or 'nodenext'. +// @ts-expect-error Module compilation target doesn't support import.metapackages/css/src/rules/index.ts (1)
Line range hint
117-824
: Excellent test coverage and structure!The test suite is comprehensive and well-organized:
- Covers all major CSS rule scenarios
- Uses proper type assertions with
satisfies StyleRule
- Includes complex nested cases
- Tests edge cases and combinations of features
Consider adding a few more test cases for:
- Error scenarios
- Edge cases with invalid inputs
packages/transform-to-vanilla/src/transform-object/index.ts (1)
Line range hint
469-824
: Comprehensive test coverage for CSS transformations!The test suite thoroughly covers:
- Basic transformations
- Complex nested scenarios
- CSS variable handling
- Media query combinations
- Selector combinations
A few suggestions for additional test cases:
- Invalid CSS property combinations
- Malformed selectors
- Performance benchmarks for large CSS objects
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
.yarn/releases/yarn-4.5.0.cjs
is excluded by!**/.yarn/**
.yarn/releases/yarn-4.5.3.cjs
is excluded by!**/.yarn/**
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (29)
.yarnrc.yml
(1 hunks)configs/eslint-config-custom/package.json
(1 hunks)configs/vite-config-custom/package.json
(1 hunks)examples/react-swc/package.json
(1 hunks)package.json
(2 hunks)packages/css/package.json
(1 hunks)packages/css/src/css/index.ts
(2 hunks)packages/css/src/rules/index.ts
(1 hunks)packages/css/src/rules/types.ts
(1 hunks)packages/css/src/rules/utils.ts
(1 hunks)packages/debug-log/src/index.ts
(1 hunks)packages/transform-to-vanilla/package.json
(1 hunks)packages/transform-to-vanilla/src/index.ts
(1 hunks)packages/transform-to-vanilla/src/transform-keys/at-rules.ts
(1 hunks)packages/transform-to-vanilla/src/transform-keys/complex-selectors.ts
(1 hunks)packages/transform-to-vanilla/src/transform-keys/css-var.ts
(1 hunks)packages/transform-to-vanilla/src/transform-keys/merge-key.ts
(1 hunks)packages/transform-to-vanilla/src/transform-keys/simple-pseudo-selectors.ts
(1 hunks)packages/transform-to-vanilla/src/transform-object/index.ts
(1 hunks)packages/transform-to-vanilla/src/transform-object/rule-context.ts
(1 hunks)packages/transform-to-vanilla/src/transform-object/variant-reference.ts
(1 hunks)packages/transform-to-vanilla/src/transform-values/css-var.ts
(1 hunks)packages/transform-to-vanilla/src/transform-values/merge-values.ts
(1 hunks)packages/transform-to-vanilla/src/transform-values/property-reference.ts
(1 hunks)packages/transform-to-vanilla/src/transform-values/simply-important.ts
(1 hunks)packages/transform-to-vanilla/src/transform.ts
(1 hunks)packages/transform-to-vanilla/src/types/string.ts
(1 hunks)packages/transform-to-vanilla/src/types/style-rule.ts
(1 hunks)packages/transform-to-vanilla/src/types/utils.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- configs/vite-config-custom/package.json
🧰 Additional context used
📓 Learnings (1)
packages/css/src/rules/index.ts (1)
Learnt from: black7375
PR: mincho-js/mincho#113
File: packages/css/src/index.ts:33-33
Timestamp: 2024-11-12T04:02:13.081Z
Learning: `recipe` is properly exported from './rules' in `packages/css/src/rules/index.ts`.
🔇 Additional comments (30)
.yarnrc.yml (1)
17-17
: Yarn updated to version 4.5.3
The yarnPath
has been updated to use Yarn version 4.5.3
, ensuring consistency with the packageManager
field in package.json
and benefiting from the latest updates and bug fixes.
packages/transform-to-vanilla/src/index.ts (2)
1-1
: Explicitly exporting the transform
function
Changing from a wildcard export to explicitly exporting the transform
function improves module encapsulation and makes the public API clearer by exposing only the intended functionality.
1-1
: Verify no other exports from ./transform
are required
By limiting the exports to only transform
, ensure that no other functions or variables from ./transform
are needed elsewhere in the codebase to prevent potential breakage.
Run the following script to check for other imports from ./transform
:
✅ Verification successful
Export limitation to transform
is appropriate
Looking at the source file transform.ts
, the only exported symbol is the transform
function. All other functions (isClassNames
) and types are internal utilities. The codebase search confirms there are no other imports of ./transform
outside of index.ts
. The export limitation is correct and won't cause any breakage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find any imports from './transform' outside of 'src/index.ts'
rg 'from\s+["'']\.\/transform["'']' -g '!src/index.ts'
Length of output: 157
Script:
#!/bin/bash
# Search for imports from transform file with proper escaping and both quote types
rg "from ['\"]\\./transform['\"]" -g '!src/index.ts'
# Also check for any other potential import patterns
rg "import.*from ['\"]\\./transform['\"]" -g '!src/index.ts'
rg "require\\(['\"]\\./transform['\"]\\)" -g '!src/index.ts'
# Let's also check what's actually exported from transform.ts
cat packages/transform-to-vanilla/src/transform.ts
Length of output: 8566
configs/eslint-config-custom/package.json (1)
20-21
: Development dependencies updated
The devDependencies have been updated to their latest versions, which is good for staying current with improvements and security fixes.
Also applies to: 24-25, 27-27
examples/react-swc/package.json (1)
17-34
: Upgraded development dependencies
The development dependencies have been updated, ensuring compatibility with the latest tooling and libraries, which enhances project stability and takes advantage of new features.
packages/transform-to-vanilla/src/transform-values/simply-important.ts (1)
Line range hint 19-33
: Well-structured test suite with good coverage!
The test cases effectively cover:
- Default case without important
- Exclamation mark handling
- Space handling
- Existing !important handling
packages/transform-to-vanilla/src/transform-keys/css-var.ts (1)
Line range hint 29-42
: Comprehensive test coverage with good positive/negative assertions!
The test suite effectively validates:
- CSS variable key detection with both formats ($var and --var)
- Proper conversion of variable names
- Negative cases for non-variable keys
packages/transform-to-vanilla/package.json (1)
64-64
: Verify @vanilla-extract/css update compatibility.
The update from ^1.15.5 to ^1.16.1 is a minor version bump. However, we should verify:
- Any breaking changes in the changelog
- Compatibility with other @vanilla-extract/* packages in the project
✅ Verification successful
Let me gather more information about the package's usage and potential breaking changes.
Based on the gathered information, I can now provide the final response:
@vanilla-extract/css update appears safe to proceed
The version bump from ^1.15.5 to ^1.16.1 is a minor update, and our analysis shows:
- The package is used consistently across the codebase for core functionality (styles, keyframes, fileScope)
- Other @Vanilla-Extract packages are in sync (vite-plugin at ^4.0.18, esbuild-plugin at ^2.3.12)
- No recent commit history indicates breaking changes or issues
- Usage patterns show type imports and core API usage that's stable across minor versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other @vanilla-extract dependencies and their versions
echo "Checking for other @vanilla-extract dependencies..."
fd -t f "package.json" -x jq -r '.dependencies + .devDependencies | with_entries(select(.key | startswith("@vanilla-extract"))) | to_entries[] | "\(.key): \(.value)"' {} \;
# Check if the package is actually used in the codebase
echo -e "\nChecking usage patterns..."
rg -l "@vanilla-extract/css" --type ts
Length of output: 1089
Script:
#!/bin/bash
# Check the git history for the package.json changes
echo "Checking package.json changes..."
git diff HEAD^1 packages/transform-to-vanilla/package.json
# Check for any failing tests or build errors in recent commits
echo -e "\nChecking recent commit messages for issues..."
git log --oneline -n 10 --grep="fail\|break\|fix.*vanilla"
# Look for specific vanilla-extract API usage patterns
echo -e "\nChecking specific API usage patterns..."
rg "@vanilla-extract/css" -B 2 -A 2 --type ts
Length of output: 4417
packages/css/package.json (1)
79-79
: Verify @vanilla-extract/css version bump implications
The version bump from ^1.15.5 to ^1.16.1 is a minor update. Let's verify its stability and changes.
✅ Verification successful
Version bump from ^1.15.5 to ^1.16.1 is safe to proceed
The changes between versions are well-documented and include:
- 1.16.1 (patch): Fixes a bug in selector generation
- 1.16.0 (minor): Adds support for
::-webkit-calendar-picker-indicator
and introducescreateViewTransition
API
All changes are backward compatible and the version is stable (currently latest on npm). The bug fix in 1.16.1 is particularly beneficial for preventing invalid selector generation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check @vanilla-extract/css version details and changelog
# Get latest version info
echo "Latest version info:"
curl -s https://registry.npmjs.org/@vanilla-extract/css/latest | jq '{version, time}'
# Get changelog between versions
echo -e "\nChangelog between versions:"
gh api repos/vanilla-extract-css/vanilla-extract/releases | jq -r '.[] | select(.tag_name | contains("@vanilla-extract/css@1.16")) | {tag_name, body}'
Length of output: 2006
packages/transform-to-vanilla/src/transform-keys/complex-selectors.ts (1)
Line range hint 31-63
: LGTM! Comprehensive test coverage
The test suite effectively covers:
- Selector type checks
- Complex selector scenarios
- Nested selector transformations
- Edge cases with multiple selectors
packages/transform-to-vanilla/src/types/string.ts (1)
28-30
: Consider adding @ts-expect-error instead of @ts-ignore
Similar to the previous file, using @ts-expect-error
would be more explicit about the expected error.
Also applies to: 32-33
packages/transform-to-vanilla/src/transform-keys/merge-key.ts (1)
Line range hint 24-69
: LGTM! Comprehensive test coverage
The test implementation is well-structured with:
- Concurrent test execution using describe.concurrent
- Clear test case organization
- Good coverage of edge cases for both functions
package.json (2)
43-64
: Verify peer dependency compatibility
Several packages have been updated. Please ensure:
- All peer dependencies are satisfied
- Test suite passes with the new versions
- No deprecation warnings are present
✅ Verification successful
This script will help us:
- Find all package.json files
- Extract their dependencies, peerDependencies, and devDependencies
- Allow us to analyze potential version conflicts or peer dependency issues
After analyzing the package.json files across the repository, I can now provide a final response:
✅ Peer dependencies are compatible with updated versions
The peer dependency requirements are minimal and flexible:
eslint-config-custom
requires any version ofeslint
(satisfied by^9.16.0
)vite-config-custom
requires any version ofvite
(satisfied by^6.0.2
)@mincho-js/css
requires any version of@vanilla-extract/css
(satisfied by^1.16.1
)
All peer dependencies use *
as version specifier except where specified above, making them compatible with the updated versions in the root package.json.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential peer dependency issues
fd -e json$ | while read -r file; do
if [[ "$file" == *"package.json" ]]; then
echo "=== $file ==="
jq -r '.peerDependencies // empty' "$file"
fi
done
Length of output: 30
Script:
#!/bin/bash
# Let's try a different approach to check package.json files and their dependencies
find . -name "package.json" -type f -exec sh -c '
echo "=== $1 ==="
cat "$1" | jq -r "{ name: .name, dependencies: .dependencies, peerDependencies: .peerDependencies, devDependencies: .devDependencies }"
' sh {} \;
Length of output: 5049
59-59
:
Upgrading from vite 5.x to 6.x may introduce breaking changes. Please ensure:
- All vite plugins are compatible with vite 6
- Build configurations are updated if needed
- Review the migration guide: https://vitejs.dev/guide/migration
packages/transform-to-vanilla/src/transform-object/variant-reference.ts (2)
32-37
: Duplicate TypeScript ignore pattern detected
This file uses the same TypeScript ignore pattern for import.meta as merge-key.ts. Consider using a shared utility as suggested earlier.
Line range hint 39-83
: LGTM! Well-structured test cases
The test implementation provides:
- Good error case coverage
- Complex variant reference testing
- Clear test descriptions
packages/transform-to-vanilla/src/transform-keys/simple-pseudo-selectors.ts (1)
42-44
: LGTM! Consistent handling of CommonJS compatibility.
The added TypeScript ignore comments follow the established pattern for handling import.meta in CommonJS compilation, maintaining consistency across the codebase.
packages/transform-to-vanilla/src/transform-values/property-reference.ts (1)
51-56
: LGTM! Consistent TypeScript configuration.
The added TypeScript ignore comments maintain consistency with other files in handling import.meta for CommonJS compatibility.
packages/transform-to-vanilla/src/types/utils.ts (1)
65-70
: LGTM: Well-structured test setup with proper TypeScript handling
The implementation correctly handles TypeScript's import.meta restrictions when compiling to CommonJS, following best practices for test setup.
packages/transform-to-vanilla/src/transform-keys/at-rules.ts (1)
71-76
: LGTM: Proper test setup with TypeScript compatibility
The implementation correctly handles TypeScript module compatibility issues.
packages/css/src/css/index.ts (1)
101-106
: LGTM: Proper test setup
The implementation correctly handles TypeScript module compatibility.
packages/transform-to-vanilla/src/transform-values/merge-values.ts (2)
80-82
: LGTM! Appropriate handling of CommonJS compilation.
The added comments properly handle TypeScript errors when compiling to CommonJS, which is a common requirement for maintaining compatibility across different module systems.
84-84
: Excellent test coverage with comprehensive scenarios!
The test suite thoroughly covers various use cases including:
- Single arrays
- Multi-value fallback arrays
- Fallback groups
- Edge cases
Also applies to: 144-144
packages/debug-log/src/index.ts (2)
144-146
: LGTM! Appropriate handling of CommonJS compilation.
The added comments properly handle TypeScript errors when compiling to CommonJS.
Line range hint 153-238
: Well-documented test cases with clear output examples!
The test cases are thoroughly documented with expected outputs in comments, which is excellent for maintainability and understanding the behavior of each function.
packages/css/src/rules/utils.ts (2)
82-84
: LGTM! Appropriate handling of CommonJS compilation.
The added comments properly handle TypeScript errors when compiling to CommonJS.
Line range hint 89-190
: Excellent test coverage with type-safe examples!
The test suite is comprehensive and well-structured:
- Covers edge cases (empty objects, undefined inputs)
- Includes type-safe variant combinations
- Tests complex scenarios with mixed types
packages/css/src/rules/types.ts (1)
Line range hint 178-577
: Excellent test implementation with comprehensive type coverage.
The test suite demonstrates:
- Well-organized test structure using concurrent describes
- Thorough coverage of variant types and pattern options
- Clear validation of type constraints
- Good use of type assertions and expectations
This implementation serves as a good example for other type testing suites in the project.
packages/css/src/rules/index.ts (1)
117-124
: LGTM: TypeScript ignore comments are properly documented.
The comments clearly explain why the TypeScript errors are being ignored, which is necessary for CommonJS compatibility when using import.meta
.
packages/transform-to-vanilla/src/transform-object/index.ts (1)
469-474
: LGTM: Consistent TypeScript ignore comments.
The comments follow the same pattern as in rules/index.ts
, maintaining consistency across the codebase.
/fast-forward |
Triggered from #134 (comment) by @black7375. Trying to fast forward Target branch ( commit d0991f539e651abf074aea91ca2dcd3078a0cd08 (HEAD -> main, origin/main, origin/HEAD)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sat Nov 23 22:51:35 2024 +0900
Fix: Rules - Prop expression type limitation #131 Pull request ( commit 842689aca2986f939ab0436c44976c01567a4544 (pull_request/bump)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Sun Dec 1 19:56:38 2024 +0900
Chore(deps): Bump packages Fast forwarding $ git push origin 842689aca2986f939ab0436c44976c01567a4544:main
To https://github.com/mincho-js/mincho.git
d0991f5..842689a 842689aca2986f939ab0436c44976c01567a4544 -> main |
Description
Bump packages
Related Issue
Summary by CodeRabbit
New Features
transform
function to handle a wider range of input types, including arrays of styles.Bug Fixes
Tests
Chores
Additional context
Checklist