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.

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

Cannot get property in an array of objects #178

Open
AmauryD opened this issue Oct 4, 2022 · 3 comments
Open

Cannot get property in an array of objects #178

AmauryD opened this issue Oct 4, 2022 · 3 comments

Comments

@AmauryD
Copy link
Contributor

AmauryD commented Oct 4, 2022

Hello,

I'm trying to get a property in an array of objects. It seems that the changes are not retrieved properly with .get(). Sadly, I don't know enough about this library to identify the problem.

Also, i didn't see examples of changeset with an array of objects in the documentation. Is it recommended ?

I made a simple reproduction test that explains the problem.

 describe('arrays of objects', () => {
    test('changes in array are gettable', () => {
      let initialData = { contacts: [] };

      const changeset = Changeset(initialData);

      changeset.set("contacts", [{
        name :'william'
      }]);

// returns undefined
      expect(changeset.get("contacts.0")).toStrictEqual({
        name :'william'
      });
      expect(changeset.get("contacts.0.name")).toStrictEqual(
        'william'
      );
    });
  });

node version : 14.19.1
validated-changeset: 1.3.3

Thank you in advance.

@AmauryD
Copy link
Contributor Author

AmauryD commented Oct 4, 2022

I dug a bit in the code, by changing this line it seems to work and the tests still passes. It is just a quickfix btw.

https://github.com/validated-changeset/validated-changeset/blob/132b516328a7faa9a243f4e81e28f3bd81138115/src/index.ts#L1019

this.isObject(normalizedBaseChanges) || Array.isArray(normalizedBaseChanges)

The fix is in my fork (https://github.com/AmauryD/validated-changeset)

@snewcomer
Copy link
Collaborator

@AmauryD Would you like putting up a PR with your changes?

@AmauryD
Copy link
Contributor Author

AmauryD commented Oct 18, 2022

@AmauryD Would you like putting up a PR with your changes?

It is done in #179 ! 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants