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.

Sign up for GitHub

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

Jump to bottom

Allow to pass domain as an argument for createEvent and similar methods #763

Merged
merged 7 commits into from
Aug 20, 2022
Merged

Allow to pass domain as an argument for createEvent and similar methods #763

merged 7 commits into from
Aug 20, 2022

Conversation

sergeysova
Copy link
Collaborator

  • Flatten config of unit not inside initUnit
  • Start implementation of the {domain} in create* methods
  • Refactor naming in createDomain
  • Call domain hooks if domain passed as config argument

Closes #590

Conventions

@sergeysova sergeysova changed the title Flatten config of unit not inside initUnit Allow to pass domain as an argument for createEvent and similar methods Aug 18, 2022
@sergeysova sergeysova marked this pull request as ready for review August 18, 2022 16:52
@sergeysova sergeysova self-assigned this Aug 18, 2022
@sergeysova sergeysova added the enhancement New feature or request label Aug 18, 2022
@sergeysova sergeysova added this to the effector 23.0.0 milestone Aug 18, 2022
AlexandrHoroshih
AlexandrHoroshih requested changes Aug 18, 2022
View reviewed changes
src/effector/createDomain.ts Outdated Show resolved Hide resolved
AlexandrHoroshih
AlexandrHoroshih approved these changes Aug 19, 2022
View reviewed changes
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.

Awesome 👍

src/effector/__tests__/domain.test.ts
}).toThrowErrorMatchingInlineSnapshot(
`"Domain cannot be created with two domains as parent. Please, remove {domain} argument."`,
)
// @ts-expect-error There is no types for case, just runtime check
Copy link
Member

@AlexandrHoroshih AlexandrHoroshih Aug 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Probably the other way around 🤔
there is no runtime check for the case, only typecheck

igorkamyshev
igorkamyshev approved these changes Aug 19, 2022
View reviewed changes
Copy link
Member

@igorkamyshev igorkamyshev left a comment

Choose a reason for hiding this comment

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

LGTM

@zerobias zerobias removed this from the effector 23.0.0 milestone Aug 20, 2022
zerobias
zerobias approved these changes Aug 20, 2022
View reviewed changes
Copy link
Member

@zerobias zerobias left a comment

Choose a reason for hiding this comment

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

Cool, thanks 👍👍 This will go to next minor release as this is not a breaking change and there is a need to publish some features earlier

@zerobias zerobias merged commit 97e7c37 into master Aug 20, 2022
@zerobias zerobias deleted the feat/allow-to-pass-domain-to-config branch August 20, 2022 16:15
@sergeysova sergeysova mentioned this pull request Dec 21, 2022
Closed
@kireevmp kireevmp mentioned this pull request May 11, 2023
Merged
sergeysova added a commit that referenced this pull request Jun 10, 2023
@sergeysova
docs: Add domain property for createStore
7b1221d 
Implement by #763
iposokhin pushed a commit to iposokhin/effector-forms that referenced this pull request Dec 10, 2023
Simplify the way form units are created
a8bd772 
Since effector v22.4.0 we can pass a domain as an argument to units factories. This allows us to get rid of the createFormUnits factory and make units creation easier. See more details effector/effector#763
@iposokhin iposokhin mentioned this pull request Dec 10, 2023
Open
@kireevmp kireevmp mentioned this pull request Feb 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zerobias zerobias zerobias approved these changes

@igorkamyshev igorkamyshev igorkamyshev approved these changes

@AlexandrHoroshih AlexandrHoroshih AlexandrHoroshih approved these changes

Assignees

@sergeysova sergeysova

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rework domain-related APIs
4 participants
@sergeysova @zerobias @igorkamyshev @AlexandrHoroshih