schema-utils icon indicating copy to clipboard operation
schema-utils copied to clipboard

feat: format object properties with types

Open vankop opened this issue 6 years ago • 13 comments

This PR contains a:

  • [ ] bugfix
  • [x] new feature
  • [ ] code refactor
  • [ ] test update
  • [ ] typo fix
  • [ ] metadata update

Motivation / Use-Case

https://github.com/webpack/schema-utils/issues/42 2nd proposal

Breaking Changes

no

Additional Info

Looks like for array and object we can skip formatting type.

vankop avatar Sep 28 '19 11:09 vankop

Codecov Report

Merging #59 (db7e4f6) into master (62fb107) will decrease coverage by 0.58%. The diff coverage is 97.67%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #59      +/-   ##
==========================================
- Coverage   98.72%   98.13%   -0.59%     
==========================================
  Files           5        5              
  Lines         550      645      +95     
  Branches      250      268      +18     
==========================================
+ Hits          543      633      +90     
- Misses          7       12       +5     
Impacted Files Coverage Δ
src/ValidationError.js 97.68% <97.33%> (-0.67%) :arrow_down:
src/index.js 100.00% <100.00%> (ø)
src/keywords/absolutePath.js 100.00% <100.00%> (ø)
src/util/Range.js 100.00% <100.00%> (ø)
src/validate.js 100.00% <100.00%> (ø)

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 62fb107...db7e4f6. Read the comment docs.

codecov[bot] avatar Sep 28 '19 12:09 codecov[bot]

Yes objects and arrays are not expanded

vankop avatar Sep 28 '19 12:09 vankop

Multiple types could be only with anyOf | oneOf. Here we just look on type if it exists

vankop avatar Sep 28 '19 12:09 vankop

so @evilebottnawi what we need in this PR still? Tests on additionalProperties ?

vankop avatar Sep 28 '19 16:09 vankop

@vankop all is good, i will review deeply this in near future

alexander-akait avatar Sep 28 '19 16:09 alexander-akait

/cc @vankop need rebase, also i think we should improve output using \n (like prettier do with objects :smile: ), because some error is very long

alexander-akait avatar Nov 07 '19 16:11 alexander-akait

also i think we should improve output using \n

@evilebottnawi you have any ideas how to do it better?

When I did this I was thinking about it, but did not realized how to do it better, since it depends totally on terminal window size + font type/size. Approach when we rely only on amount of properties also fails because of glyph sizes

vankop avatar Nov 11 '19 20:11 vankop

@vankop the good question, maybe we can solve this in other PR, i think packages like table have algorithm for this, need look on them logic

alexander-akait avatar Nov 12 '19 10:11 alexander-akait

/cc @evilebottnawi

Ready to review

maybe we can solve this in other PR, i think packages like table have algorithm for this, need look on them logic

I think this really important, I will take a look in table package, thanks for suggestion

vankop avatar Nov 26 '19 10:11 vankop

Also interesting question is - do we need sort properties alphabetically?

vankop avatar Nov 26 '19 11:11 vankop