Change dynamic field interfaces
mizdra opened this issue · 5 comments
Motivation
- The
get
function does not return a usable type.- The type of
get('author')
isPromise<OptionalAuthor | undefined>
. OptionalAuthor
means{ id?: string | undefined, name?: string | undefined, email?: string | undefined }
.- All fields of
author
may beundefined
. This is inconvenient for the user. - Therefore,
get
function is not very useful.
- The type of
- Dependent Field is not simple to define.
- What it means...
- Call
get(...)
:get('name')
- Add
await
keyword beforeget(...)
:await get('name')
- Prepare default value:
(await get('name')) ?? 'defaultName'
- Compute the field:
`${(await get('name')) ?? 'defaultName'}@yuyushiki.net`
- Wrap with
dynamic
function:dynamic(async ({ get }) => `${(await get('name')) ?? 'defaultName'}@yuyushiki.net`)
- Call
- Many steps are required. It is not simple.
- I would like to define it more simply.
- What it means...
Proposal
Change the interface as follows:
Case. 1: Use seq
in defaultFields
and .build()
function
Before
const BookFactory = defineBookFactory({
defaultFields: {
id: dynamic(({ seq }) => `Book-${seq}`),
name: 'Book',
},
});
const book = await BookFactory.build({
name: dynamic(({ seq }) => `Book-${seq}`),
});
After
const BookFactory = defineBookFactory({
defaultFields: (({ seq }) => ({
id: `Book-${seq}`,
name: 'Book',
})),
});
const book = await BookFactory.build(({ seq }) => ({
name: `Book-${seq}`,
}));
Case. 2: Use get
for Dependent Fields
Before
const AuthorFactory = defineAuthorFactory({
defaultFields: {
name: 'mikamikomata',
email: dynamic(async ({ get }) => `${(await get('name')) ?? 'defaultName'}@yuyushiki.net`),
},
});
After
const AuthorFactory = defineAuthorFactory({
defaultFields: (({ seq }) => {
const name = 'mikamikomata';
return {
name,
email: `${name}@yuyushiki.net`,
};
}),
});
Case. 3: Use get
for Transient Fields (depend: #73)
Before
const AuthorFactory = defineAuthorFactory.withTransientFields({
bookCount: 0,
})({
defaultFields: {
books: dynamic(async ({ get }) => {
const bookCount = (await get('bookCount')) ?? 0;
return BookFactory.buildList(bookCount);
}),
},
});
After
const AuthorFactory = defineAuthorFactory.withTransientFields({
bookCount: 0,
})({
defaultFields: async ({ transientFields: { bookCount } }) => ({
books: await BookFactory.buildList(bookCount),
}),
});
Breaking Changes
- Remove
get
function - Remove
dynamic
utility
Please let me know if this should be a separate issue, but with the proposed changes: could the factories (maybe optionally) become synchronous? Looking at one example, since there's nothing async involved in defineXFactory
I would assume that the value generated from .build()
could also be synchronous:
const AuthorFactory = defineAuthorFactory({
defaultFields: (({ seq }) => {
const name = 'mikamikomata';
return {
name,
email: `${name}@yuyushiki.net`,
};
}),
});
That might make it easier to reuse the factories in more places, e.g.: Storybook Stories where the args
are declared synchronously.
@YellowKirby Thanks for your interest in this issue.
could the factories (maybe optionally) become synchronous?
What does factories
mean? Do you mean AuthorFactory
? Or do you mean .build()
method?
What does factories mean? Do you mean AuthorFactory? Or do you mean .build() method?
Sorry, I meant the .build()
(and .buildList()
) method
OK.
However, making the build method synchronous is outside the scope of this issue. In this issue, the build method is asynchronous.
I understand the need for a synchronous build method in an environment where top-level await is not available. I have considered implementing that functionality in the past. But I gave up. I did not want to complicate the internal code.
Therefore, I recommend migrating to an ESM that allows for top-level await. Recently many tools support it these days. Storybook also support it (storybookjs/storybook#15129).
Okie dokie! That sounds reasonable, thank you! I think the top-level await will cover what I'm going for in Storybook, thanks for the link :)