refactor: replace assert with with #579
Conversation
WalkthroughThe changes in this pull request primarily involve updating the import syntax for JSON files across multiple test files. The import statements have been modified from using Changes
Poem
Warning Rate limit exceeded@TobiTenno has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 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 (
|
|
Code Climate has analyzed commit 37b1cf8 and detected 0 issues on this pull request. View more on Code Climate. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (7)
safeImport.js (1)
Line range hint
1-7: Consider adding Node.js version requirement to JSDocSince this function now uses the
withimport attribute syntax, it would be helpful to document the minimum Node.js version requirement in the JSDoc comment./** * Safely require path, fall back to fallback if module cannot load + * @requires Node.js 20.x or later for import attributes support * @param {string} path Path to attempt to load * @param {Object} fallback fallback response if error or no module * @returns {Promise<any> module or the default object */test/missionTypes.spec.js (1)
Line range hint
1-38: Consider adding JSDoc comments for better documentation.While the test file is well-structured, it would benefit from JSDoc comments describing the purpose of the test suite and the schema definition.
Add documentation like this:
+/** + * Test suite for mission types data validation. + * Ensures that the mission types JSON file adheres to the expected schema. + */ +/** + * Schema for validating mission type entries. + * @type {Object} + * @property {Object} definitions - Schema definitions + * @property {Object} patternProperties - Validates keys starting with 'MT_' + */ const missionTypesSchema = {test/archonShards.spec.js (1)
The import path is correct, but the test path needs to be updated
The import statement correctly uses
../data/archonShards.json(relative to the test directory), but the test assertion uses./data/archonShards.jsonwhich is incorrect. The test file is in thetestdirectory, so using./datawould look for the file intest/datawhich doesn't exist.
- Update the test path in
test/archonShards.spec.jsline 31 from'./data/archonShards.json'to'../data/archonShards.json'to match the import path🔗 Analysis chain
Line range hint
31-33: Verify the JSON file path in the test.There's an inconsistency between the import path (
../data/archonShards.json) and the test path (./data/archonShards.json). This could potentially cause issues.Let's verify the correct file path:
Let me check the actual test file content to understand the full context of imports and test structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and location of archonShards.json # Expected: Find the correct path to ensure tests are checking the right file fd archonShards.jsonLength of output: 383
Script:
#!/bin/bash # Check the content of the test file cat test/archonShards.spec.jsLength of output: 840
test/tutorials.spec.js (1)
5-5: Consider documenting Node.js version requirement.Since the
withsyntax for import attributes is a newer feature, it would be helpful to document the minimum Node.js version requirement in the README or package.json.Add the minimum Node.js version to package.json:
{ "engines": { + "node": ">=16.14.0" } }test/arcanes.spec.js (1)
Line range hint
27-27: Fix typo in schema property name.There's a typo in the URI format validator:
foramtshould beformat.Apply this fix:
- thumbnail: { type: 'string', foramt: 'uri' }, + thumbnail: { type: 'string', format: 'uri' },test/languages.spec.js (1)
13-13: Consider refactoring to use static importThe change from
asserttowithis correct. However, since this import is used for a constant value, consider using a static import instead of a dynamic one for better code clarity and potentially better performance.-const syndicates = Object.keys(await import('../data/syndicatesData.json', { with: { type: 'json' } })); +import syndicatesData from '../data/syndicatesData.json' with { type: 'json' }; +const syndicates = Object.keys(syndicatesData);test/conclaveData.spec.js (1)
5-5: Consider documenting Node.js version requirement.Since the Import Attributes syntax (
with { type: 'json' }) requires a modern Node.js version, it would be helpful to document this requirement.Add a comment above the import statement:
+// Requires Node.js version that supports Import Attributes (replaces Import Assertions) import conclaveData from '../data/conclaveData.json' with { type: 'json' };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (17)
safeImport.js(1 hunks)test/arcanes.spec.js(1 hunks)test/archonShards.spec.js(1 hunks)test/conclaveData.spec.js(1 hunks)test/eventsData.spec.js(1 hunks)test/factionsData.spec.js(1 hunks)test/fissureModifiers.spec.js(1 hunks)test/languages.spec.js(1 hunks)test/missionTypes.spec.js(1 hunks)test/operationTypes.spec.js(1 hunks)test/persistentEnemyData.spec.js(1 hunks)test/solNodes.spec.js(1 hunks)test/sortieData.spec.js(1 hunks)test/syndicatesData.spec.js(1 hunks)test/synthTargets.spec.js(1 hunks)test/tutorials.spec.js(1 hunks)test/upgradeTypes.spec.js(1 hunks)
🔇 Additional comments (15)
test/upgradeTypes.spec.js (1)
5-5: LGTM! Modernized import syntax.
The change from assert to with for JSON module imports aligns with modern JavaScript practices.
Let's verify this change is consistent across all test files:
✅ Verification successful
All JSON imports consistently use modern with syntax
The verification shows that:
- No instances of
assertJSON imports remain in the test directory - All JSON imports consistently use the modern
with { type: 'json' }syntax - One slight variation exists in
languages.spec.jsusing dynamic import with{ with: { type: 'json' } }, which is also the correct modern syntax for dynamic imports
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining 'assert' JSON imports and verify all JSON imports use 'with'
# Check for any remaining 'assert' JSON imports
echo "Checking for remaining 'assert' JSON imports:"
rg "assert.*type.*json" test/
# Verify consistent usage of 'with' JSON imports
echo "Verifying 'with' JSON imports:"
rg "with.*type.*json" test/
Length of output: 1993
test/syndicatesData.spec.js (1)
5-5: LGTM! The import statement has been correctly updated.
The change from assert to with follows the modern JSON module import syntax.
Let's verify the Node.js version compatibility for this syntax:
✅ Verification successful
Import statement update is correctly implemented and consistent across the codebase
The verification confirms:
- Node.js version requirement is
>=18.19.0, which supports thewithsyntax for JSON imports - All test files consistently use the new
with { type: 'json' }syntax - No remaining
asserttype JSON imports were found - The change is part of a complete migration across all test files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check package.json for Node.js version requirements and verify compatibility with 'with' imports
# Check if package.json exists and get Node.js version
if [ -f "package.json" ]; then
echo "Node.js version requirements:"
jq '.engines.node' package.json
fi
# Search for other similar import statements to ensure consistency
echo "Checking other test files for import consistency:"
rg "import.*\.json['\"]\s*(assert|with)\s*{.*}" "test/"
Length of output: 467
Script:
#!/bin/bash
# Fix the regex and search for JSON imports in test files
echo "Checking test files for JSON imports:"
rg "import.*\.json.*with.*{.*type.*:.*'json'.*}" test/
# Check if there are any remaining assert-type JSON imports
echo -e "\nChecking for any remaining assert-type JSON imports:"
rg "import.*\.json.*assert.*{.*type.*:.*'json'.*}" test/
# List all test files to ensure we haven't missed any JSON imports
echo -e "\nAll test files in the repository:"
fd "\.spec\.[tj]s$" test/
Length of output: 1312
Script:
#!/bin/bash
# Search for JSON imports with fixed regex
echo "Checking for JSON imports in test files:"
rg -l "import.*from.*json.*with" test/
rg -l "import.*from.*json.*assert" test/
# Get content of a few test files to verify the import pattern
echo -e "\nSampling test files for JSON import patterns:"
for file in test/arcanes.spec.js test/factionsData.spec.js test/syndicatesData.spec.js; do
echo -e "\n=== $file ==="
head -n 10 "$file"
done
Length of output: 2076
test/persistentEnemyData.spec.js (1)
5-5: LGTM! The import syntax update follows modern JavaScript practices.
The change from assert to with for JSON imports aligns with the latest JavaScript module import assertions syntax. This is a forward-looking change that maintains compatibility while adopting more modern practices.
Let's verify this change is consistent across the codebase:
✅ Verification successful
All JSON imports have been consistently updated to use the modern with syntax
The verification shows:
- No remaining instances of
assertstyle JSON imports were found - All JSON imports across the codebase consistently use the modern
with { type: 'json' }syntax - The change is uniformly applied in both direct imports and dynamic imports (as seen in
safeImport.js)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining assert-style JSON imports that might need updating
# and verify the new syntax is used consistently.
# Check for any remaining assert-style JSON imports
echo "Checking for remaining assert-style JSON imports:"
rg "assert.*type.*json" --type js
# Check for consistent usage of with-style JSON imports
echo "Verifying consistent with-style JSON imports:"
rg "with.*type.*json" --type js
Length of output: 2135
test/factionsData.spec.js (1)
5-5: LGTM! Appropriate use of modern import syntax.
The change from assert to with for JSON module imports aligns with the latest JavaScript standards. This is a safe change as it's purely syntactical and doesn't affect the functionality of the tests.
Let's verify that this change is consistent across all test files:
✅ Verification successful
Import syntax consistently updated across all test files
The verification shows that all JSON imports in test files are using the modern with { type: 'json' } syntax consistently. There are no remaining instances of the old assert syntax, and the change in factionsData.spec.js aligns with the pattern used across all 16 test files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of 'assert { type: "json" }' in test files
# and confirm all JSON imports use the new 'with' syntax
echo "Checking for any remaining 'assert' JSON imports..."
rg "assert\s*{\s*type:\s*['\"]json['\"]" "test/"
echo "Verifying consistent use of 'with' JSON imports..."
rg "with\s*{\s*type:\s*['\"]json['\"]" "test/"
Length of output: 587
Script:
#!/bin/bash
# Let's fix the regex syntax and try again
echo "Checking for any remaining 'assert' JSON imports..."
rg "assert *{ *type: *['\"]json['\"]" test/
echo "Verifying consistent use of 'with' JSON imports..."
rg "with *{ *type: *['\"]json['\"]" test/
# Also let's check all JSON imports in test files to ensure we haven't missed any patterns
echo "Checking all JSON imports in test files..."
rg "import.*from.*\.json" test/
Length of output: 2324
test/archonShards.spec.js (1)
5-5: LGTM! Good modernization of import syntax.
The change from assert to with for JSON imports aligns with modern JavaScript practices. The with syntax is the newer standard for import attributes in ECMAScript.
Let's verify if there are any remaining instances of assert imports that should be updated:
✅ Verification successful
Import syntax is consistently modern across the codebase
All JSON imports in the test files are already using the modern with { type: 'json' } syntax. There are no remaining instances of the older assert syntax that need to be updated.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining 'assert' import statements that might need updating
# Expected: No instances of 'assert { type: "json" }' should remain
rg "assert.*type.*json" --type js
Length of output: 35
Script:
#!/bin/bash
# Let's try a broader search to find any JSON imports
rg "import.*from.*\.json['\"]" -A 1 --type js
Length of output: 2142
test/fissureModifiers.spec.js (1)
5-5: LGTM! Import assertion syntax successfully updated.
The change from assert to with aligns with the latest ECMAScript import attributes proposal, which is replacing import assertions.
Let's verify the consistency of this change across other test files:
✅ Verification successful
Import assertion syntax consistently updated across the codebase
The verification shows that:
- No instances of the old
assertsyntax remain for JSON imports - The new
withsyntax is consistently used across all test files and utilities - There are two valid patterns in use:
- Static imports:
import x from 'y.json' with { type: 'json' } - Dynamic imports:
import(path, { with: { type: 'json' } })
- Static imports:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of 'assert { type: "json" }' in import statements
# and confirm all JSON imports use the new 'with' syntax
# Check for any remaining assert usage in imports
echo "Checking for remaining 'assert' usage in imports:"
rg "import.*assert.*json"
# Verify consistent usage of new 'with' syntax
echo "Verifying 'with' syntax usage:"
rg "import.*with.*json"
Length of output: 2101
test/tutorials.spec.js (1)
5-5: LGTM! Verify Node.js compatibility.
The change from assert to with for import attributes follows modern JavaScript practices. However, this syntax requires a compatible Node.js version.
Let's verify the Node.js version requirements in the project:
✅ Verification successful
Node.js version requirement is sufficient for import attributes
The package.json specifies Node.js >=18.19.0 which supports the new with import attribute syntax. This version requirement is adequate as import attributes with with syntax were introduced in Node.js 18.19.0.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check Node.js version requirements
# Expected: package.json should specify minimum Node.js version that supports import attributes with 'with'
# Check if package.json exists and contains engines field
if [ -f "package.json" ]; then
echo "Current Node.js version requirements:"
jq '.engines.node' package.json
else
echo "package.json not found"
fi
Length of output: 154
test/operationTypes.spec.js (1)
5-5: LGTM! The import syntax update looks correct.
The change from assert to with aligns with modern JavaScript's JSON module imports. This is a good improvement as the with syntax is the standardized approach.
Let's verify the Node.js version compatibility across the project:
✅ Verification successful
Import syntax update is consistent and compatible
The verification confirms:
- Node.js version requirement (>=18.19.0) supports JSON imports with
withsyntax - All JSON imports across test files consistently use the modern
with { type: 'json' }syntax - No legacy
asserttype JSON imports found in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if package.json specifies a Node.js version that supports the 'with' syntax for JSON imports
# Check package.json for Node.js version requirements
rg -A 5 '"engines":|"node":' package.json
# Look for any other JSON imports to ensure consistency
rg "import.*\.json['\"]\s*(assert|with)"
Length of output: 1818
test/arcanes.spec.js (1)
5-5: LGTM! Import syntax updated correctly.
The change from assert to with for JSON module imports aligns with modern ES modules best practices.
test/eventsData.spec.js (2)
5-5: LGTM! Good modernization of import syntax.
The change from assert to with aligns with the newer ECMAScript standard for import attributes. This is a safe refactoring that maintains the same functionality while using more modern syntax.
5-5: Verify consistent usage across test files.
Let's ensure this syntax update has been applied consistently across all test files.
✅ Verification successful
Import syntax is consistently updated across test files
The verification shows that:
- No test files are using the old
assertimport syntax - All JSON imports in test files are using the new
with { type: 'json' }syntax - There's only one slight variation in
languages.spec.jswhich uses dynamic import with the same type assertion
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining uses of 'assert' in import statements
# and verify consistent use of 'with' across test files
echo "Checking for any remaining 'assert' imports in test files..."
rg "import.*assert.*json" test/
echo -e "\nVerifying consistent use of 'with' imports in test files..."
rg "import.*with.*json" test/
Length of output: 2091
test/conclaveData.spec.js (1)
5-5: LGTM! The import statement correctly uses the new 'with' syntax.
The change from assert to with aligns with the TC39 Import Attributes proposal, which is the successor to Import Assertions.
Let's verify the Node.js version compatibility:
✅ Verification successful
Import Attributes syntax is compatible with specified Node.js version
The package.json requires Node.js >=18.19.0 and .nvmrc specifies lts/hydrogen (v18.x), which both support the Import Attributes proposal with the with syntax. The change is safe to use with the project's Node.js requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if package.json specifies a Node.js version compatible with Import Attributes
# Check for package.json engine requirements
if [ -f "package.json" ]; then
echo "Node.js version specified in package.json:"
cat package.json | jq '.engines.node'
fi
# Look for .nvmrc or .node-version files
if [ -f ".nvmrc" ]; then
echo "Node.js version in .nvmrc:"
cat .nvmrc
elif [ -f ".node-version" ]; then
echo "Node.js version in .node-version:"
cat .node-version
fi
Length of output: 279
test/solNodes.spec.js (1)
5-5: LGTM! Good modernization of import syntax.
The change from assert to with follows the latest JavaScript modules specification and is the recommended approach for import attributes.
Let's verify this pattern is consistently applied across other test files:
✅ Verification successful
Import syntax modernization is consistently applied across all test files
The verification confirms that:
- No instances of the older
assertsyntax remain in any test files - The newer
withsyntax is consistently used across all 16 test files for JSON imports
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of assert imports
# and confirm consistent usage of with imports across test files
echo "Checking for any remaining assert imports..."
rg "assert \{ type: 'json' \}" test/
echo "Confirming consistent with imports..."
rg "with \{ type: 'json' \}" test/
Length of output: 1891
test/sortieData.spec.js (1)
5-5: LGTM! Import statement updated correctly.
The change from assert to with for JSON module imports aligns with the PR objectives and maintains the same functionality.
Let's verify similar changes across other test files mentioned in the summary:
✅ Verification successful
All JSON imports have been consistently updated to use with syntax
The verification shows that:
- No remaining
asserttype JSON imports were found in test files - All test files are using the new
withsyntax for JSON imports - One slight variation exists in
languages.spec.jsusing dynamic import, but it still correctly uses thewithtype specification
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all test files have been updated to use 'with' instead of 'assert'
# for JSON imports consistently.
# Search for any remaining 'assert' type JSON imports in test files
echo "Checking for any remaining 'assert' type JSON imports..."
rg "import.*assert.*type.*json" "test/"
# Verify that 'with' type JSON imports are used consistently
echo "Verifying 'with' type JSON imports..."
rg "import.*with.*type.*json" "test/"
Length of output: 2045
test/synthTargets.spec.js (1)
5-5: LGTM! Good modernization of import syntax.
The change from assert to with follows the latest ES modules best practices, as the assert syntax was experimental and is being replaced by the standardized with syntax.
Let's verify if there are any remaining files that need similar updates:
✅ Verification successful
All JSON imports are already using the modern with syntax
Based on the search results, all JSON imports across the codebase are consistently using the modern with { type: 'json' } syntax. No files are using the deprecated assert syntax, so no further changes are needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find any remaining JSON imports using the deprecated 'assert' syntax
# Expected: No results, as all files should be using the new 'with' syntax
# Search for JSON imports using assert syntax
rg "import.*from.*\.json.*assert.*{.*type:.*'json'.*}"
# Search for JSON imports to ensure we haven't missed any
rg "import.*from.*\.json"
Length of output: 1873
|
🎉 This PR is included in version 2.16.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What did you fix?
replace assert with with
Reproduction steps
Evidence/screenshot/link to line
Considerations
Summary by CodeRabbit
assert { type: 'json' }towith { type: 'json' }, improving module handling.foramttoformatinfactionsData.spec.js.These changes enhance the reliability of JSON imports throughout the testing suite.