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: dynamic object declaration #120

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

josh-hemphill
Copy link

closes #102
All previous tests are passing, and relevant ones were made.
It seems to work reasonably well from what I can tell so far, I just emulated the behavior of how Arrays are handled to create an object key wildcard.
This would definitely have to be a major version change since there's a good chance this will break some people's implementation.

I also want to apologize for the ESLint changes, I have global ESLint and I was trying to prevent my global config from making sweeping stylistic changes.

@codecov
Copy link

codecov bot commented Aug 1, 2020

Codecov Report

Merging #120 into master will increase coverage by 0.10%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #120      +/-   ##
==========================================
+ Coverage   98.79%   98.89%   +0.10%     
==========================================
  Files           6        6              
  Lines         248      271      +23     
==========================================
+ Hits          245      268      +23     
  Misses          3        3              
Impacted Files Coverage Δ
src/property.js 100.00% <100.00%> (ø)
src/schema.js 100.00% <100.00%> (ø)
src/utils.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 ed3f1ba...0c3be13. Read the comment docs.

@josh-hemphill
Copy link
Author

I also had to create an object typecast function, which might not have ideal behavior, and probably should be in the same module the other typecasts come from; let me know if you want me to create a pull request for typecast or how I might improve the function.

@josh-hemphill
Copy link
Author

josh-hemphill commented Aug 3, 2020

I added tests for the two functions I missed.
I realized I shouldn't have modified the eslintrc, so I reverted that but had to add .vscode to the .gitignore.

@eivindfjeldstad Everything passes now and maintains code coverage.
Are there any tests you'd like me to add or major changes you'd like me to make?
Is this such a breaking change I should put it behind some kind of config in the instantiation of schemas? Or do you think it's consistent enough with the rest of the library's behavior it should just go with as is with a new semver major version increment?

@oychao
Copy link

oychao commented Jan 15, 2021

Will this get merged and published soon? desperately needed.

@josh-hemphill
Copy link
Author

josh-hemphill commented Jan 15, 2021

Will this get merged and published soon? desperately needed.

There were compounding changes I needed, so I ended up creating josh-hemphill/valivar which is written in typescript and has dictionary/dynamic objects. It borrows heavily from this repo because I love the API.
So it should be compatible with code written for validate. Though I'm still working on the documentation.

Oh, and valivar also supports Deno

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.

Dynamic property support
2 participants