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

Support nested nullable objects #203

Open
kettanaito opened this issue Apr 8, 2022 · 3 comments · May be fixed by #231
Open

Support nested nullable objects #203

kettanaito opened this issue Apr 8, 2022 · 3 comments · May be fixed by #231
Labels
feature New feature or request good for beginners Issue suitable for a new contributor.

Comments

@kettanaito
Copy link
Member

kettanaito commented Apr 8, 2022

This has to be supported:

const db = factory({
  user: {
    id: primaryKey(String),
    permissions: nullable(() => ({
      id: String,
    })),
  },
})

But it's not. Recursive nullable objects as well. No primary key needed. An API without a getter function would be preferable: nullable({ ... }).

@kettanaito kettanaito added good for beginners Issue suitable for a new contributor. feature New feature or request labels Apr 8, 2022
@aloysb
Copy link

aloysb commented May 21, 2022

@kettanaito I'll give this a crack 😉

aloysb pushed a commit to aloysb/data that referenced this issue May 22, 2022
aloysb pushed a commit to aloysb/data that referenced this issue May 22, 2022
node 'node_modules/.bin/jest' '/Users/aloysberger/FOSS/data/test/model/create.test.ts' -c
'/Users/aloysberger/FOSS/data/test/jest.config.ts' -t 'supports nested nullable object' --runInBand

re mswjs#203
aloysb pushed a commit to aloysb/data that referenced this issue May 22, 2022
…bjects

Update tests to cover nested nullable objects

re mswjs#203
@aloysb
Copy link

aloysb commented May 22, 2022

See PR #219 👍 😃

@yishayweb
Copy link
Contributor

Hi guys,

I opened a new PR for solving this issue with an api that doesn't use a nullable getter as @kettanaito asked
Can someone please take a look at it here ?

@kettanaito @abitbetterthanyesterday please let me know if this solution is acceptable and if so I will add some notes about it in the documentation

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request good for beginners Issue suitable for a new contributor.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants