TNG / ngqp

Declaratively synchronize form controls with the URL
https://tng.github.io/ngqp
MIT License
81 stars 8 forks source link

[Solution #1] fix(core): Improve typing and support emptyOn for multi:true #102

Closed Airblader closed 5 years ago

Airblader commented 5 years ago

This change improves the type safety by not misinterpreting an array-valued parameter as a multi: true parameter. We also introduce a new MultiQueryParam type as a shortcut.

As a side effect, we move the evaluation of (de-)serializing entirely to the service and can thus now support emptyOn for multi: true parameters.

fixes #92 fixes #27

Signed-off-by: Ingo Bürk ingo.buerk@tngtech.com

I have verified that…

codecov[bot] commented 5 years ago

Codecov Report

Merging #102 into master will decrease coverage by 0.11%. The diff coverage is 89.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #102      +/-   ##
==========================================
- Coverage   83.96%   83.85%   -0.12%     
==========================================
  Files          25       25              
  Lines         655      675      +20     
  Branches       99      105       +6     
==========================================
+ Hits          550      566      +16     
- Misses         52       54       +2     
- Partials       53       55       +2
Impacted Files Coverage Δ
projects/ngqp/core/src/lib/serializers.ts 66.66% <ø> (-3.34%) :arrow_down:
...re/src/lib/directives/query-param-group.service.ts 82.85% <85.71%> (-1.24%) :arrow_down:
projects/ngqp/core/src/lib/model/query-param.ts 91.48% <85.71%> (-1.99%) :arrow_down:
...s/ngqp/core/src/lib/query-param-builder.service.ts 96.66% <95.45%> (+2.54%) :arrow_up:

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 78ac98b...8202c83. Read the comment docs.

alexey-anufriev commented 5 years ago

Looking forward to having this merged.

Airblader commented 5 years ago

Closing in favor of #105