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

Expose a replayed value$ observable #177

Open
Airblader opened this issue Dec 10, 2020 · 2 comments
Open

Expose a replayed value$ observable #177

Airblader opened this issue Dec 10, 2020 · 2 comments
Labels
Comp: Core Core functionality of ngqp good first issue Good for newcomers Priority: Medium Users can work without this for now, but this should be fixed soon-ish Type: Feature If you want to add something that doesn't exist yet

Comments

@Airblader
Copy link
Collaborator

What's your idea?
QueryParam and QueryParamGroup currently expose a valueChanges observable, akin to Angular Forms, which is backed by a Subject<>. This can create a problem as the first value may be "missed", see #72 and #176. While there is a relatively easy solution to this, it still creates unnecessary boilerplate and can be confusing.

Therefore, we should expose a value$ observable on both which is backed by a ReplaySubject<>(1). This makes it easier for users to connect ngqp to their logic.
@Airblader Airblader added good first issue Good for newcomers Type: Feature If you want to add something that doesn't exist yet Comp: Core Core functionality of ngqp Priority: Medium Users can work without this for now, but this should be fixed soon-ish labels Dec 10, 2020
@Airblader Airblader mentioned this issue Dec 10, 2020
Closed
@Airblader
Copy link
Collaborator Author

As a side note, in the documentation we should prioritize & encourage the use of value$ and discourage that of valueChanges.

@w5l
Copy link

w5l commented Nov 2, 2022

Moving users away from valueChanges implicitly means moving the project away from following the Angular Forms style. Using startsWith(...) fixes the issue and when needed in multiple locations, it can be moved into a wrapper class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Comp: Core Core functionality of ngqp good first issue Good for newcomers Priority: Medium Users can work without this for now, but this should be fixed soon-ish Type: Feature If you want to add something that doesn't exist yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Airblader @w5l