fix: [Angular] fix logic used for parsing objects provided as input to useObjectWrapper

[lukjab71]

Description

This is propsed fix to issue: 1410

• What changes you made:
  The logic responsible for parsing an input to the method was incorrect. It used ',' comma as object separator. This gave weird results for any other constructs that would use comma, like arrays. Besides, this logic should probably be used only when input actually has spread operator in it.

Regardless from object detection logic, I'm not convinced if logic responsible for concatenation ${spreadOutObjects.join(', ')}, ${otherObjs.join(', ')} was correct. spreadOutObjects collects objects that use spread operator. otherObjs collect the rest of the objects from input. When we concatenate them using the initial logic, I believe we are completely mixing the order of objects, as there is no guarantee that spreadOutObjects in the original input, where at the begining. What if they were spread out somewhere else, in some nested object, of some complicated input? I believe this would either completely malform the object or in best case change the object structure.

Now, about the propsed solution. As I understand, the whole purpose of this method is to get rid of ... spread operator from syntax, as angular won't accept it.

I tried to approach this problem in recursive way(to fix initial way of detecting objects - instead of using comma as object separator, try to figure start bracket and end bracket), but it is tricky, as regex are not good with recursive problems and even when I extracted recursion to separate method, it still was tricky for some complicated examples. And then I switched the approach - in the end, why do we need to detect separate objects? Our goal is to get rid of ... from the string. So we can simply get rid of the ... and then deal with potential consequences:

1. someField: ...someObject -> remove ... and we are done
2. someField: {...someObject} -> remove ... and we are left with {object} result
3. someField: {...{nestedDeeper: true }} -> remove ... and we are left with {{ something: true }} result

I believe those cases will cover every possible variations on different levels of nesting. So after removing ... we just have to cleanup what's left.

I prepared the PR using remarks from CONTRIBUTING.md file, however, I noticed that even without my changes there are tests failing in the core package(?).

[changeset-bot]

🦋 Changeset detected

Latest commit: c5e5b77

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@builder.io/mitosis	Patch
@builder.io/mitosis-cli	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit c5e5b77. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

---

✅ Successfully ran 4 targets

• nx run-many --target test
• nx e2e @builder.io/e2e-app
• nx run-many --target build --exclude @builder.io/e2e-alpine
• nx build @builder.io/mitosis-site

---

Sent with 💌 from NxCloud.

[sidmohanty11]

@lukjab71 thanks for the PR! just wanted to understand how this will handle spread values, can you please add a test for that? As far as I understand this will just remove ... from any values and let useObjectWrapper handle it right?

you can use this for test:

import { useStore } from "@builder.io/mitosis";

export default function MyComponent(props) {
  const state = useStore({
    attributes: {id:1},
    attributes2: {id2:1},
    something: {id3:1},
  });

  return (
    <div>
      <Comp
        val1={{...state.attributes2}}
        val2={{
          ...state.attributes,
          ...state.attributes2,
        }}
        val3={{
          ...state.something,
         anything: 'hello',
          hello: 'world', 
        }}
        val4={{
          ...state.attributes,
          ...state.something,
          anything: [1,2,3],
          hello: 'hello',
          ...state.attributes2,
        }}
        val5={{
          ...state.attributes,
          ...state.something,
          anything: [1,2,3],
          hello: 'hello',
          ...props.spreadAttrs,
        }}
        />
    </div>
  );
}

[lukjab71]

Yes, this is how I understood the initial logic for useObjectWrapper, that had the bug. Remove spread operator to input parameter, provided to useObjectWrapper function, to avoid using spread syntax for angular components inputs.

I updated the raw file and snaps with propsed example.

To be honest I don't fully understand the testing strategy in the repo. Even without my changes, when I run test command for core package I get:

Test Files 5 failed | 37 passed | 1 skipped (43)
Tests 48 failed | 5914 passed | 3 skipped (5965)

Those failures are completely unrelated to my changes. I hope that in general everything is working as expected.

[sidmohanty11]

thank you for explaining, looks good to me!

[sidmohanty11]

can you please try to pull the latest main and run yarn test:update from root and see if it works?

[sidmohanty11]

might have to lint the test file too

[lukjab71]

I've merged with current main branch, I've run yarn test:update. It went through with success. I used prettier and lint command.

[sidmohanty11]

im curious if this will actually work? this should fail right? because we cannot send params like this?

[lukjab71]

Good remark. My solution was too generic and it messed up the parameters. I updated it. It reuses both concepts - initial and mine.

Spread operator is removed. If the object was presented in form of {...obj}, we replace it with obj. If there was some spread inside of object, we get rid of spread and deal with consequences. {...obj1,test:true, ...obj2} => obj1, {test: true}, obj2.

When I look at generated snaps, they seem to be ok.

[sidmohanty11]

same thing here, will we be able to pass params like this?

[lukjab71]

Good remark. My solution was too generic and it messed up the parameters. I updated it. It reuses both concepts - initial and mine.

Spread operator is removed. If the object was presented in form of {...obj}, we replace it with obj. If there was some spread inside of object, we get rid of spread and deal with consequences. {...obj1,test:true, ...obj2} => obj1, {test: true}, obj2.

When I look at generated snaps, they seem to be ok.

[sidmohanty11]

looks awesome!

[sidmohanty11]

could you please add some comments for each regex, explaining the purpose behind it for future reference?

[lukjab71]

Ofc, I added comments for each step of this operation in 801cf8bf57

[sidmohanty11]

LGTM! Thank you very much for your contribution @lukjab71!