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

feat(core): support asynchronous deserializers #131

Merged
merged 1 commit into from
Apr 26, 2019
Merged

Conversation

Airblader
Copy link
Collaborator

@Airblader Airblader commented Apr 12, 2019

relates #93

This is a work in progress following a quick proof of concept. Open points I need to work on:

  • Tests
  • Documentation
  • Documentation: Explicitly mention we look at the first emission
  • Documentation: Mention that valueChanges fires after the deserializer
  • Actually first() on the returned observable + takeUntil
  • Ensure the rxjs pipeline has proper intermediate types
  • forkJoin is currently highlighted as deprecated due to the wrong type being used; fix this

This pull request relates to or closes the following issues:

I have verified that…

  • the commits in this pull request follow the conventionalcommits pattern
  • tests have been updated or added
  • documentation has been updated to reflect the changes made

@Airblader Airblader force-pushed the feature-93 branch 3 times, most recently from 4722ca3 to c7261a1 Compare April 12, 2019 12:53
@Airblader Airblader marked this pull request as ready for review April 26, 2019 07:05
We now support deserializers which run asynchronously by returning an
observable instead. This is useful when the deserializer relies on some
asynchronous process or data which has to be fetched first.

relates #93

Signed-off-by: Ingo Bürk <[email protected]>
@codecov
Copy link

codecov bot commented Apr 26, 2019

Codecov Report

Merging #131 into master will increase coverage by 0.16%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #131      +/-   ##
==========================================
+ Coverage    84.6%   84.77%   +0.16%     
==========================================
  Files          25       25              
  Lines         734      742       +8     
  Branches      115      117       +2     
==========================================
+ Hits          621      629       +8     
  Misses         54       54              
  Partials       59       59
Impacted Files Coverage Δ
projects/ngqp/core/src/lib/util.ts 95% <100%> (+0.4%) ⬆️
projects/ngqp/core/src/lib/model/query-param.ts 91.2% <75%> (+0.09%) ⬆️
...re/src/lib/directives/query-param-group.service.ts 84.31% <93.75%> (-0.26%) ⬇️
...s/multi-select-control-value-accessor.directive.ts 87.75% <0%> (+2.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9fb8ed5...428e266. Read the comment docs.

@Airblader Airblader merged commit 2fc8051 into master Apr 26, 2019
@Airblader Airblader deleted the feature-93 branch April 26, 2019 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant