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.

Sign up for GitHub

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

Jump to bottom

fix: revert #300

Merged
merged 1 commit into from
Oct 11, 2024
Merged

fix: revert #300

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 26 additions & 76 deletions packages/state/src/utils/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,10 @@
import { Type as RecsType, Schema } from "@dojoengine/recs";

export function convertValues(schema: Schema, values: any) {
if (typeof schema !== "object" || schema === null) {
console.warn("Invalid schema provided.");
return {};
}

if (typeof values !== "object" || values === null) {
console.warn("Invalid values provided.");
return {};
}

return Object.keys(schema).reduce<any>((acc, key) => {
if (!acc) {
acc = {};
}

const schemaType = schema[key];
const value = values[key];

Expand All @@ -31,30 +20,26 @@

switch (schemaType) {
case RecsType.StringArray:
if (value.type === "array") {
if (value.value.length === 0) {
acc[key] = [];
} else if (value.value[0].type === "enum") {
acc[key] = value.value.map(
(item: any) => item.value.option
);
} else {
acc[key] = value.value.map((a: any) => {
try {
return BigInt(a.value);
} catch (error) {
console.warn(
`Failed to convert ${a.value} to BigInt. Using string value instead.`
);
return a.value;
}
});
}
} else {
console.warn(
`Expected type 'array' for key '${key}', but received '${value.type}'.`
if (value.type === "array" && value.value.length === 0) {
acc[key] = [];
} else if (
value.type === "array" &&
value.value[0].type === "enum"
) {
Comment on lines +26 to +28
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add a check for non-empty array before accessing value.value[0]

In the condition on lines 26-28, you're accessing value.value[0].type without ensuring that value.value is not an empty array. This could lead to an error if value.value is an empty array. Consider adding a check to ensure value.value.length > 0 before accessing the first element.

Apply this diff to fix the issue:

} else if (
    value.type === "array" &&
+   value.value.length > 0 &&
    value.value[0].type === "enum"
) {
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
value.type === "array" &&
value.value[0].type === "enum"
) {
value.type === "array" &&
value.value.length > 0 &&
value.value[0].type === "enum"
) {

acc[key] = value.value.map(
(item: any) => item.value.option
);
acc[key] = undefined;
} else {
acc[key] = value.value.map((a: any) => {
try {
return BigInt(a.value);
} catch (error) {
console.warn(
`Failed to convert ${a.value} to BigInt. Using string value instead.`
);
return a.value;
}
});
}
break;

Expand All @@ -67,16 +52,10 @@
acc[key] = BigInt(value.value);
} catch (error) {
console.warn(
`Failed to convert ${value.value} to BigInt. Attempting hexadecimal conversion.`
`Failed to convert ${value.value} to BigInt. Using string value instead.`
);
try {
acc[key] = BigInt(`0x${value.value}`);
} catch (hexError) {
console.warn(
`Failed to convert ${value.value} to BigInt even with hexadecimal. Assigning undefined.`
);
acc[key] = undefined;
}

acc[key] = BigInt(`0x${value.value}`);

Check failure on line 58 in packages/state/src/utils/index.ts

View workflow job for this annotation

GitHub Actions / check

src/__tests__/utils.test.ts > convertValues > BigInt conversion fallback > should fallback to hexadecimal conversion for invalid BigInt strings 

SyntaxError: Cannot convert 0xinvalid_bigint to a BigInt
 ❯ src/utils/index.ts:58:32
 ❯ Module.convertValues src/utils/index.ts:4:32
 ❯ src/__tests__/utils.test.ts:156:28
Comment on lines +55 to +58
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Align warning message with error handling logic

The warning message indicates that the string value will be used instead of a BigInt, but the code attempts to parse the value as a hexadecimal string and convert it to BigInt. This inconsistency might cause confusion.

Please update the warning message to reflect the actual behavior:

console.warn(
-    `Failed to convert ${value.value} to BigInt. Using string value instead.`
+    `Failed to convert ${value.value} to BigInt. Attempting to parse as hexadecimal.`
);

Alternatively, if the intention is to use the string value directly when parsing fails, adjust the code accordingly:

try {
    acc[key] = BigInt(value.value);
} catch (error) {
    console.warn(
        `Failed to convert ${value.value} to BigInt. Using string value instead.`
    );
    acc[key] = value.value;
}

Committable suggestion was skipped due to low confidence.

}
break;

Expand All @@ -93,46 +72,17 @@
if (value.value instanceof Map) {
const structValues = Object.fromEntries(value.value);
acc[key] = convertValues(schemaType, structValues);
} else if (
typeof value.value === "object" &&
value.value !== null
) {
acc[key] = convertValues(schemaType, value.value);
} else {
console.warn(
`Expected 'struct' type with object value for key '${key}'.`
);
acc[key] = undefined;
acc[key] = convertValues(schemaType, value.value);
}
} else if (
Array.isArray(schemaType) &&
value.type === "array"
) {
if (!Array.isArray(value.value)) {
console.warn(
`Expected an array for key '${key}', but received '${typeof value.value}'.`
);
acc[key] = undefined;
} else {
acc[key] = value.value.map((item: any) => {
if (
item.type === "struct" &&
typeof schemaType[0] === "object"
) {
return convertValues(schemaType[0], item.value);
} else {
// Handle non-struct items or different types as needed
return convertValues(
typeof schemaType[0] === "object"
? schemaType[0]
: {},
item
);
}
});
}
acc[key] = value.value.map((item: any) =>
convertValues(schemaType[0], item)
);
Comment on lines +82 to +84
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure schemaType array is not empty before accessing schemaType[0]

In the array handling logic, you're accessing schemaType[0] without checking if schemaType has any elements. If schemaType is an empty array, this will result in undefined, potentially causing errors in convertValues. Consider adding a check to ensure schemaType.length > 0 before accessing schemaType[0].

Apply this diff to fix the issue:

} else if (
    Array.isArray(schemaType) &&
+   schemaType.length > 0 &&
    value.type === "array"
) {
    acc[key] = value.value.map((item: any) =>
        convertValues(schemaType[0], item)
    );
}

Committable suggestion was skipped due to low confidence.

} else {
// For unrecognized schema types, assign the raw value
acc[key] = value.value;
}
break;
Expand Down
Loading