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

feat: support primitive arrays in models #98

Closed

Conversation

with-heart
Copy link

Closes #97

This PR adds primitive arrays support to models which is discussed in #97.

Changes made to support this feature:

  • created BaseTypesArray type
  • added BaseTypesArray to ModelDefinition and Limit
  • added check for primitive arrays in createModel to use the exact array value rather than executing propertyDefinition()
  • added check for object entities to manyOf block in removeInternalProperties so that primitive array properties are left alone

This is my first time exploring @mswjs/data, so I could use some feedback on the approach here.

I also ran into an issue where I was having to run yarn build in order for the tests to use the updated code. Is that typical?

src/glossary.ts Outdated Show resolved Hide resolved
@motakbiri2
Copy link

@with-heart @kettanaito is there anything here that I can help with to make this PR ready to go?

@kettanaito
Copy link
Member

kettanaito commented Sep 8, 2021

Currently, the tests are failing here:

FAIL test/model/create.test.ts
  ● creates a new entity

    TypeError: Cannot use 'in' operator to search for '__type' in with-heart

       8 | function isInternalEntity(value) {
       9 |     return (value &&
    > 10 |         glossary_1.InternalEntityProperty.type in value &&
         |                                                ^
      11 |         glossary_1.InternalEntityProperty.primaryKey in value);
      12 | }
      13 | exports.isInternalEntity = isInternalEntity;

      at isInternalEntity (lib/utils/isInternalEntity.js:10:48)
          at Array.every (<anonymous>)
      at isManyOfRelation (lib/utils/removeInternalProperties.js:26:42)
      at lib/utils/removeInternalProperties.js:46:13
          at Array.reduce (<anonymous>)
      at Object.removeInternalProperties (lib/utils/removeInternalProperties.js:32:35)
      at Object.create (lib/factory.js:73:47)
      at Object.<anonymous> (test/model/create.test.ts:18:30)

This failure is caused by the isInternalEntity function being unable to process Arrays as entity values:

export function isInternalEntity(
value: Record<string, any>,
): value is InternalEntity<any, any> {
return (
value &&
InternalEntityProperty.type in value &&
InternalEntityProperty.primaryKey in value
)
}

So evaluating "__type" in [1, 2, 3] results in a type error (and rightfully so). I think we should extend isInternalEntity to understand arrays as entity values. But given the expectations of the function, those being that the entity is expected to be an object, it makes more sense to include the object assertion instead. That's something that's introduced in #113 already, and we may want to merge these changes in the right order to get them in this pull request.

@kettanaito
Copy link
Member

I've introduced the object assertion in isInternalEntity as a separate pull request. Will rebase this feature branch once the said pull request lands.

@kettanaito
Copy link
Member

Array support in model definition was added in #113. Thank you for your work here, @with-heart!

@kettanaito kettanaito closed this Oct 12, 2021
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

Successfully merging this pull request may close these issues.

Model doesn't support array of strings, numbers or booleans
3 participants