Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix: when value of array is empty, no need for data type recalculation (#38794)

Browse Source 
## Description
<ins>Problem</ins>
When we have a `select/multi-select` field type and the source data
gives empty array to the `JSONFormWidget`, the widget tries to gauge the
sub type for value inside the array, and since it is empty it got
`undefined`

This led to re-evaluation of property config for the field. 

<ins>Solution</ins>
- Added a check to prevent unnecessary recalculation of sub data types
when arrays are empty in `checkIfArrayAndSubDataTypeChanged`.

- Changed parameter type from `any` to `unknown` in `dataTypeFor` and
`subDataTypeFor` functions for improved type safety.

- Added and refactored unit tests.

This refactor enhances type safety and optimizes performance in the
schema parsing logic.

Fixes #37246  
_or_  
Fixes `Issue URL`
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags="@tag.JSONForm"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/12905055896>
> Commit: cad7015881c16a9af273b84dcf44cc33e32fb7d9
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12905055896&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.JSONForm`
> Spec:
> <hr>Wed, 22 Jan 2025 09:50:23 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Tests**
	- Enhanced test suite for determining field types.
- Added comprehensive test cases covering primitive values, email
formats, date formats, array types, object types, and edge cases.

- **Refactor**
- Updated type annotations in schema parser functions to improve type
safety.
	- Changed parameter types from `any` to `unknown`.
	- Added clarifying comment for handling empty arrays in type checking.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Rahul Barwal 2025-01-23 15:14:49 +05:30 committed by GitHub
parent 78718229da
commit 0af11b9128
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 124 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

159
app/client/src/widgets/JSONFormWidget/schemaParser.test.ts
View File

@ -1873,54 +1873,123 @@ describe(".normalizeArrayValue", () => {
});
describe(".fieldTypeFor", () => {
it("return default field type of data passed", () => {
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const inputAndExpectedOutputs: [any, FieldType][] = [
["string", FieldType.TEXT_INPUT],
["2021-12-30T10:36:12.1212+05:30", FieldType.DATEPICKER],
["December 30, 2021 10:36 AM", FieldType.DATEPICKER],
["December 30, 2021", FieldType.DATEPICKER],
["2021-12-30 10:36", FieldType.DATEPICKER],
["2021-12-30T10:36:12", FieldType.DATEPICKER],
["2021-12-30 10:36:12 AM", FieldType.DATEPICKER],
["30/12/2021 10:36", FieldType.DATEPICKER],
["30 December, 2021", FieldType.DATEPICKER],
["10:36 AM 30 December, 2021", FieldType.DATEPICKER],
["2021-12-30", FieldType.DATEPICKER],
["12-30-2021", FieldType.DATEPICKER],
["30-12-2021", FieldType.DATEPICKER],
["12/30/2021", FieldType.DATEPICKER],
["30/12/2021", FieldType.DATEPICKER],
["30/12/21", FieldType.DATEPICKER],
["12/30/21", FieldType.DATEPICKER],
["40/10/40", FieldType.TEXT_INPUT],
["2000/10", FieldType.TEXT_INPUT],
["1", FieldType.TEXT_INPUT],
["#111", FieldType.TEXT_INPUT],
["999", FieldType.TEXT_INPUT],
["test@demo.com", FieldType.EMAIL_INPUT],
["test@.com", FieldType.TEXT_INPUT],
[10, FieldType.NUMBER_INPUT],
[[{}], FieldType.ARRAY],
[[""], FieldType.MULTISELECT],
[[1], FieldType.MULTISELECT],
[[null], FieldType.MULTISELECT],
[null, FieldType.TEXT_INPUT],
[undefined, FieldType.TEXT_INPUT],
[{ foo: "" }, FieldType.OBJECT],
[
() => {
10;
},
FieldType.TEXT_INPUT,
],
it("returns appropriate field types for primitive values", () => {
const testCases = [
{ input: "simple text", expected: FieldType.TEXT_INPUT },
{ input: 42, expected: FieldType.NUMBER_INPUT },
{ input: true, expected: FieldType.SWITCH },
{ input: null, expected: FieldType.TEXT_INPUT },
{ input: undefined, expected: FieldType.TEXT_INPUT },
];
inputAndExpectedOutputs.forEach(([input, expectedOutput]) => {
const result = fieldTypeFor(input);
testCases.forEach(({ expected, input }) => {
expect(fieldTypeFor(input)).toEqual(expected);
});
});
expect(result).toEqual(expectedOutput);
it("detects email field type correctly", () => {
const testCases = [
{ input: "valid@email.com", expected: FieldType.EMAIL_INPUT },
{ input: "user.name+tag@example.co.uk", expected: FieldType.EMAIL_INPUT },
{ input: "invalid.email@", expected: FieldType.TEXT_INPUT },
{ input: "@invalid.com", expected: FieldType.TEXT_INPUT },
{ input: "not-an-email", expected: FieldType.TEXT_INPUT },
];
testCases.forEach(({ expected, input }) => {
expect(fieldTypeFor(input)).toEqual(expected);
});
});
it("detects date field type correctly", () => {
const testCases = [
{ input: "2024-03-14", expected: FieldType.DATEPICKER },
{ input: "03/14/2024", expected: FieldType.DATEPICKER },
{ input: "14/03/2024", expected: FieldType.DATEPICKER },
{ input: "2024-03-14T15:30:00", expected: FieldType.DATEPICKER },
{ input: "March 14, 2024", expected: FieldType.DATEPICKER },
{ input: "not-a-date", expected: FieldType.TEXT_INPUT },
{ input: "99/99/9999", expected: FieldType.TEXT_INPUT },
{
input: "2021-12-30T10:36:12.1212+05:30",
expected: FieldType.DATEPICKER,
},
{ input: "December 30, 2021 10:36 AM", expected: FieldType.DATEPICKER },
{ input: "2021-12-30 10:36", expected: FieldType.DATEPICKER },
{ input: "2021-12-30 10:36:12 AM", expected: FieldType.DATEPICKER },
{ input: "30/12/2021 10:36", expected: FieldType.DATEPICKER },
{ input: "30 December, 2021", expected: FieldType.DATEPICKER },
{ input: "10:36 AM 30 December, 2021", expected: FieldType.DATEPICKER },
];
testCases.forEach(({ expected, input }) => {
expect(fieldTypeFor(input)).toEqual(expected);
});
});
it("determines array field types correctly", () => {
const testCases = [
{
input: [
{ id: 1, name: "test1" },
{ id: 2, name: "test2" },
],
expected: FieldType.ARRAY,
},
{
input: ["option1", "option2"],
expected: FieldType.MULTISELECT,
},
{
input: [1, 2, 3],
expected: FieldType.MULTISELECT,
},
{
input: [],
expected: FieldType.MULTISELECT,
},
{
input: [["nested"], ["arrays"]],
expected: FieldType.ARRAY,
},
];
testCases.forEach(({ expected, input }) => {
expect(fieldTypeFor(input)).toEqual(expected);
});
});
it("handles object field types correctly", () => {
const testCases = [
{
input: { key: "value" },
expected: FieldType.OBJECT,
},
{
input: { nested: { object: true } },
expected: FieldType.OBJECT,
},
{
input: {},
expected: FieldType.OBJECT,
},
];
testCases.forEach(({ expected, input }) => {
expect(fieldTypeFor(input)).toEqual(expected);
});
});
it("handles special cases and edge values", () => {
const testCases = [
{ input: () => {}, expected: FieldType.TEXT_INPUT },
{ input: Symbol("test"), expected: FieldType.TEXT_INPUT },
{ input: BigInt(9007199254740991), expected: FieldType.NUMBER_INPUT },
{ input: NaN, expected: FieldType.NUMBER_INPUT },
];
testCases.forEach(({ expected, input }) => {
expect(fieldTypeFor(input)).toEqual(expected);
});
});
});

17
app/client/src/widgets/JSONFormWidget/schemaParser.ts
View File

@ -183,9 +183,7 @@ export const getSourceDataPathFromSchemaItemPath = (
return sourceDataPath;
};
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const dataTypeFor = (value: any) => {
export const dataTypeFor = (value: unknown) => {
const typeOfValue = typeof value;
if (Array.isArray(value)) return DataType.ARRAY;
@ -195,13 +193,11 @@ export const dataTypeFor = (value: any) => {
return typeOfValue as DataType;
};
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const subDataTypeFor = (value: any) => {
export const subDataTypeFor = (value: unknown) => {
const dataType = dataTypeFor(value);
if (dataType === DataType.ARRAY) {
return dataTypeFor(value[0]);
return dataTypeFor((value as unknown[])[0]);
}
return undefined;
@ -352,6 +348,13 @@ export const checkIfArrayAndSubDataTypeChanged = (
const currSubDataType = subDataTypeFor(currentData);
const prevSubDataType = subDataTypeFor(prevData);
/**
* If the array is empty, then we don't need to check for sub data type
* as it would be an empty array, which will always be `undefined`.
* which leads to unnecessary re calculation of a type(https://github.com/appsmithorg/appsmith/issues/37246)
*/
if (currentData.length === 0 || prevData.length === 0) return false;
return currSubDataType !== prevSubDataType;
};