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

Allow to pass array of units for spread operator #333

Merged

Conversation

earthspacon
Copy link
Contributor

@earthspacon earthspacon commented Jul 25, 2024

Description

This PR will resolve this issue

Trigger multiple units for each target field

sample({
  clock: RoomGate.open,
  target: spread({
    roomId: [getRoomFx, setSocketFx],
    userId: [setUserIdFx, userIdChanged],
  }),
})

Checklist

  • Add tests to src/method-name/method-name.test.ts
  • Add fork tests to src/method-name/method-name.fork.test.ts
  • Add type tests to test-typings/method-name.ts
    • Use // @ts-expect-error to mark expected type error
    • import { expectType } from 'tsd' to check expected return type
  • Add documentation in src/method-name/readme.md
    • Add header Patronum/MethodName
    • Add section with overloads, if have
    • Add Motivation, Formulae, Arguments and Return sections for each overload
    • Add useful examples in Example section for each overload

@sergeysova
Copy link
Member

Hello @earthspacon!
Thank you for the contribution.
Please, add fork tests along with type tests.

@earthspacon
Copy link
Contributor Author

Hello @earthspacon! Thank you for the contribution. Please, add fork tests along with type tests.

@sergeysova @AlexandrHoroshih done

Copy link
Member

@AlexandrHoroshih AlexandrHoroshih left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@earthspacon
Copy link
Contributor Author

This commit should resolve this and this issues.

But it breaks this test case with prepend. What do you think @sergeysova @AlexandrHoroshih

@sergeysova
Copy link
Member

But it breaks this test case with prepend.

Looks like .prepend() has some issues with type inferring.
Maybe remove this test? Or modify it with support of `.prepend((value: ExactType) => {})

But I prefer to fix this test.

@sergeysova sergeysova self-assigned this Sep 25, 2024
@sergeysova sergeysova added the feature New functionality label Sep 25, 2024
@earthspacon
Copy link
Contributor Author

earthspacon commented Sep 27, 2024

Or modify it with support of `.prepend((value: ExactType) => {})

Yep, that's working, also with generics .prepend<ExactType>((value) => {}) // value is ExactType
Wrote tests for that

Copy link
Member

@sergeysova sergeysova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job!
🧡

@sergeysova sergeysova merged commit 036b2f0 into effector:main Sep 30, 2024
4 checks passed
@sergeysova sergeysova added enhancement Improvement in existing feature and removed feature New functionality labels Oct 9, 2024
@sergeysova sergeysova changed the title spread - support array of units in target fields Allow to pass array of units for spread operator Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement in existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

multiple targets for spread
3 participants