Skip to content

fix(cdk): list-template-manager should always emit latest value #1460

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

hoebbelsB
Copy link
Member

Bug

The list template manager emits an empty array after it received a value and detected no changes to render. This leads to unexpected value emissions through rxFors rendercallback.

Fix

The list template manager should always emit the latest set of data, even if no change was detected on new value input.
This is e.g. useful in scenarios where you rely on the renderCallback in order to hide a loading spinner after an asynchronous operation.
I'm thinking about a search bar triggering an http request and returning the same data as before. rxFor wouldn't render any new change, but the renderCallback should still indicate the change got processed.

@nx-cloud
Copy link

nx-cloud bot commented Nov 8, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit a8258e7. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 5 targets

Sent with 💌 from NxCloud.

@hoebbelsB hoebbelsB requested a review from BioPhoton November 8, 2022 13:54
@github-actions github-actions bot added 🛂 Test Unit tests, e2e tests, integration tests, test coverage 🛠️ CDK CDK related labels Nov 8, 2022
@github-actions github-actions bot added the </> Template @rx-angular/template related label Nov 8, 2022
@codecov
Copy link

codecov bot commented Nov 8, 2022

Codecov Report

Merging #1460 (a8258e7) into main (ec364d8) will increase coverage by 1.86%.
The diff coverage is 40.00%.

@@            Coverage Diff             @@
##             main    #1460      +/-   ##
==========================================
+ Coverage   74.95%   76.81%   +1.86%     
==========================================
  Files         151      113      -38     
  Lines        2951     2295     -656     
  Branches      531      392     -139     
==========================================
- Hits         2212     1763     -449     
+ Misses        617      429     -188     
+ Partials      122      103      -19     
Flag Coverage Δ
cdk 73.38% <40.00%> (+0.06%) ⬆️
state ?
template 90.06% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
libs/cdk/template/src/lib/list-template-manager.ts 69.44% <40.00%> (+1.83%) ⬆️
.../deprecated/transformation-helpers/object/slice.ts
libs/state/actions/src/lib/proxy.ts
...transformation-helpers/object/dictionaryToArray.ts
.../deprecated/transformation-helpers/array/remove.ts
libs/state/actions/src/lib/actions.factory.ts
...te/schematics/src/migrations/update-1.4.7/index.ts
.../deprecated/transformation-helpers/array/insert.ts
.../deprecated/transformation-helpers/object/patch.ts
...src/lib/deprecated/transformation-helpers/index.ts
... and 30 more

@edbzn
Copy link
Member

edbzn commented Nov 22, 2022

I started to investigate to find out why coverage reports are broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠️ CDK CDK related </> Template @rx-angular/template related 🛂 Test Unit tests, e2e tests, integration tests, test coverage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants