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: add support for typeof, instanceof ({Function\|RegExp}) #10

Merged

Conversation

gribnoysup
Copy link
Contributor

@gribnoysup gribnoysup commented Oct 28, 2017

Noteable Changes

A possible solution for #3. The solution is the same as in main webpack lib but I also added a typeof keyword as it also can be useful.

I also updated tests to check error behavior more thoroughly.

#3 also blocks webpack/webpack#5892

Issues

@jsf-clabot
Copy link

jsf-clabot commented Oct 28, 2017

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Oct 28, 2017

Codecov Report

Merging #10 into master will increase coverage by 0.32%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #10      +/-   ##
==========================================
+ Coverage   94.11%   94.44%   +0.32%     
==========================================
  Files           3        3              
  Lines          17       18       +1     
  Branches        3        3              
==========================================
+ Hits           16       17       +1     
  Misses          1        1
Impacted Files Coverage Δ
src/validateOptions.js 100% <100%> (ø) ⬆️

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 96525dd...2839a98. Read the comment docs.

Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Awesome thx, just nits 😛

};

expect(() => validateOptions('test/fixtures/schema.json', options, '{Name}'))
.toThrowError(/Validation Error\n\n{Name} Invalid Options\n\n/);
const test = () => validateOptions('test/fixtures/schema.json', options, '{Name}')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test => validate

.toThrowError(/Validation Error\n\n{Name} Invalid Options\n\n/);
const test = () => validateOptions('test/fixtures/schema.json', options, '{Name}')

it('should throw error', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it => test

expect(test).toThrowError(/Validation Error\n\n{Name} Invalid Options\n\n/);
})

it('should have errors for every key in options', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it => test

const errors = error.err.map(e => e.dataPath)

expect(errors).toMatchObject(expected)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add a snapshot test after .toMatchObject while you at it :)

expect(validate).toMatchSnapshot();

@michael-ciniawsky michael-ciniawsky changed the title Add support for typeof, instanceof; Update tests feat: add support for typeof, instanceof ({Function\|RegExp}) Oct 28, 2017
@michael-ciniawsky michael-ciniawsky added this to the 0.4.0 milestone Oct 28, 2017
@gribnoysup
Copy link
Contributor Author

gribnoysup commented Oct 28, 2017

@michael-ciniawsky fixed, thanks for the feedback! :)

@michael-ciniawsky
Copy link
Member

michael-ciniawsky commented Oct 28, 2017

@gribnoysup I made a request for a release, should be released as soon as someone from the release team as the time 😛

@michael-ciniawsky michael-ciniawsky removed this from the 0.5.0 milestone Oct 28, 2017
@michael-ciniawsky
Copy link
Member

Released in v0.4.0 🎉

@joshwiens
Copy link
Member

Evidently nobody has permission to publish to schema-utils right now.

NPM usually doesn't take too much time to get back to you for support requests, in the mean time you will have to use the git tag. I'll publish 0.4.0 to npm as soon as the access gets fixed.

@gribnoysup gribnoysup deleted the features/support-function-type branch October 29, 2017 07:52
@gribnoysup
Copy link
Contributor Author

@d3viant0ne @michael-ciniawsky thanks everybody for the quick feedback and support!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants