Skip to content
This repository has been archived by the owner on Jan 19, 2021. It is now read-only.

feat: add options validation (schema-utils) #65

Merged
merged 1 commit into from Jul 13, 2017
Merged

feat: add options validation (schema-utils) #65

merged 1 commit into from Jul 13, 2017

Conversation

michael-ciniawsky
Copy link
Member

@michael-ciniawsky michael-ciniawsky commented Jul 9, 2017

Blocked by #63

@mattlewis92
Copy link
Member

Good idea! This is semver major btw as the loader will throw on invalid options now: https://github.com/webpack-contrib/schema-utils/blob/master/src/validateOptions.js#L20

Copy link
Member

@mattlewis92 mattlewis92 left a comment

Choose a reason for hiding this comment

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

What about sourceMapUrlCallback?

@michael-ciniawsky
Copy link
Member Author

You're right but luckily v3.0.0 is in beta atm 😅

What about sourceMapUrlCallback?

It's a {Function} which isn't directly supported by ajv (JSON Schema) and I hadn't the time to look into a custom extension to support this type in schema-utils yet, but it is highly needed :). additionalProperties: true is the workaround meanwhile 😛

@mattlewis92
Copy link
Member

Fair enough 🙂

@mattlewis92
Copy link
Member

@michael-ciniawsky can you rebase this on master? 😄 Also might be worth adding a test that it throws on an invalid option type.

@joshwiens joshwiens removed this from the 3.0.1 milestone Jul 12, 2017
@codecov
Copy link

codecov bot commented Jul 12, 2017

Codecov Report

Merging #65 into master will increase coverage by 1.39%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #65      +/-   ##
=========================================
+ Coverage    90.9%   92.3%   +1.39%     
=========================================
  Files           2       2              
  Lines          11      13       +2     
  Branches        2       2              
=========================================
+ Hits           10      12       +2     
  Misses          1       1
Impacted Files Coverage Δ
src/index.js 91.66% <100%> (+1.66%) ⬆️

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 4525107...e516db6. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 12, 2017

Codecov Report

Merging #65 into master will increase coverage by 1.39%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #65      +/-   ##
=========================================
+ Coverage    90.9%   92.3%   +1.39%     
=========================================
  Files           2       2              
  Lines          11      13       +2     
  Branches        2       2              
=========================================
+ Hits           10      12       +2     
  Misses          1       1
Impacted Files Coverage Δ
src/index.js 91.66% <100%> (+1.66%) ⬆️

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 4525107...678a8d0. Read the comment docs.

Copy link
Member

@mattlewis92 mattlewis92 left a comment

Choose a reason for hiding this comment

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

LGTM

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

Successfully merging this pull request may close these issues.

None yet

3 participants