-
-
Notifications
You must be signed in to change notification settings - Fork 44
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
add support for event source for reshape
#295
base: main
Are you sure you want to change the base?
add support for event source for reshape
#295
Conversation
Run & review this pull request in StackBlitz Codeflow. |
🚛 size-compare reportComparing 97763341...97763341 Files wasn't changed
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, delete this change from PR. It is not related to the introduced feature and must be handled in different PR.
const tempStore = createStore<Type | null>(null); | ||
sample({ source: source, filter: Boolean, target: tempStore }); | ||
|
||
result[key] = tempStore.map(mapUnit(fn)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think, not changing the meaning of the operator, and returning an object of events if passed event? Otherwise, we need to change the type of the value (| null
) to correctly create a store for each key.
So, I don't understand the case where we need to create stores from the event. We can use restore
const shape = reshape({
source: restore(eventSource, null),
shape: {
length: (original) => original?.length ?? null,
uppercase: (original) => original?.toUpperCase() ?? null,
hasSpace: (original) => original?.includes(' ') ?? null,
},
})
Or use any other default value.
I see the case for event -> event mapping for reshape
:
const result = reshape({
source: eventSource,
shape: {
length: (original) => original.length,
uppercase: (original) => original.toUpperCase(),
hasSpace: (original) => original.includes(' '),
},
});
is.event(result.length) // true
Here we don't have to change the type of the shape (reshape shape.length
returns number
, so result.length
has the same type).
reshape
Add event source support for reshape operator