Skip to content

fix(getOptions): deprecation warn in loaderUtils #147

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Mar 7, 2017
Merged

fix(getOptions): deprecation warn in loaderUtils #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5e07d05
ci(travis): contrib standard Travis build
joshwiens Feb 25, 2017
2a3b2f9
fix(getOptions): deprecation warn in loaderUtils
joshwiens Feb 25, 2017
06fe7dd
ci(Travis): drop before script install
joshwiens Feb 25, 2017
30b9e1e
ci(Travis): add note about defualts upgrade for test suite
joshwiens Feb 25, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
yarn.lock -diff
3 changes: 2 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/node_modules
test/output/*
test/node_modules/*
test/output/*
43 changes: 33 additions & 10 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,36 @@
# Tasks are commented out purposfully. Will be uncommented for webpack-defaults application
sudo: false
language: node_js
node_js:
- "0.10"
- "0.11"
- "0.12"
- "4"
branches:
only:
- master
matrix:
fast_finish: true
include:
- os: linux
node_js: '6'
env: JOB_PART=test
- os: linux
node_js: '4.3'
env: JOB_PART=test
- os: linux
node_js: '7'
env: JOB_PART=test
# - os: linux
# node_js: '7'
# env: WEBPACK_VERSION="2.2.0" BITHOUND_CHECK=true JOB_PART=lint
# - os: linux
# node_js: '7'
# env: WEBPACK_VERSION="2.2.0" JOB_PART=coverage
before_install:
- nvm --version
- node --version
before_script:
# - if [ "$WEBPACK_VERSION" ]; then yarn add webpack@^$WEBPACK_VERSION; fi
# - if [ "$BITHOUND_CHECK" ]; then npm install -g bithound; bithound check git@github.com:$TRAVIS_REPO_SLUG.git; fi
script:
- "npm test"
# test with npm@3
- "rm -rf node_modules && npm install npm@3"
- "node_modules/.bin/npm install"
- "node_modules/.bin/npm test"
- yarn run travis:$JOB_PART
Copy link
Member

@jhnns jhnns Mar 1, 2017

Choose a reason for hiding this comment

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

Why yarn? Our primary goal should still be npm.

Copy link
Member Author

@joshwiens joshwiens Mar 1, 2017

Choose a reason for hiding this comment

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

Travis uses yarn across the entire org though we do not force the use of yarn for users.
Hence the extra command below. With the coming defaults upgrade, more parts will be added to the matrix.

Copy link
Member Author

@joshwiens joshwiens Mar 1, 2017

Choose a reason for hiding this comment

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

For users that don't want to use yarn, the lock file is completely benign ( it's treated as a binary file in git )

Copy link
Member

@jhnns jhnns Mar 1, 2017

Choose a reason for hiding this comment

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

Ok ... and could you describe what this change in the travis.yml was about? It's not related to the loader-utils change and I haven't worked with travis matrices so far, so I'm a bit puzzled :)

I don't want to bother you, I just want to understand the change 😁

Copy link
Member Author

@joshwiens joshwiens Mar 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The loader-utils update forces node 4.3 as a minimum supported version ( old travis file tests against 0.10.x & 0.12.x). Given I had to update it anyway, I grabbed the travis file that will be added as part of the pending webpack-defaults upgrade ( it's used in other libs already ).

So while it's not directly related, it is a side effect of the forced drop of the older node versions.

Copy link
Member Author

@joshwiens joshwiens Mar 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

If you look at the two commented out portions of the matrix "lint & coverage" this setup also allows us to test against older / newer versions of webpack by adding another include with a specific WEBPACK_VERSION on whatever node version we want. These can also be set to Can Fail in the matrix for testing against a beta build / nightly without failing the pull request outright.

WEBPACK_VERSION="2.2.0" is removed from the three test parts becuase the test suite fails schema validation. Hence the comment on the bottom of the file.

# after_success:
# - bash <(curl -s https://codecov.io/bash)

# TODO: This test suite will throw schema errors with webpack 2.2.x installed. Address this on defaults upgrade to Jest
2 changes: 1 addition & 1 deletion index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ var trailingSlash = /[\\\/]$/;

module.exports = function(source) {
var loaderContext = this;
var query = loaderUtils.parseQuery(this.query);
var query = loaderUtils.getOptions(this) || {};
var cb = this.async();
var isSync = typeof cb !== "function";
var finalCb = cb || this.callback;
Expand Down
5 changes: 3 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"author": "Tobias Koppers @sokra",
"description": "less loader module for webpack",
"scripts": {
"test": "node --no-deprecation node_modules/.bin/_mocha -R spec",
"test": "npm run travis:test",
"travis:test": "node --no-deprecation node_modules/.bin/_mocha -R spec",
Copy link
Member

@jhnns jhnns Mar 1, 2017

Choose a reason for hiding this comment

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

  • Why is there an extra command for travis:test? We should just define test
  • I know that this has been there before, but why --no-deprecation?

Copy link
Member Author

@joshwiens joshwiens Mar 1, 2017

Choose a reason for hiding this comment

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

Why is there an extra command for travis:test? We should just define test

Look at the .travis.yml. The matrix executes different parts across different supported node versions. A part is defined as travis:<part>

I know that this has been there before, but why --no-deprecation

I have no idea, left it alone mainly becuase I don't know why it was there in the first place.

I can either remove it now or it will be removed on the defaults upgrade to Jest. Your choice.

"test-source-map": "webpack --config test/sourceMap/webpack.config.js && open ./test/sourceMap/index.html"
},
"peerDependencies": {
Expand All @@ -31,6 +32,6 @@
}
],
"dependencies": {
"loader-utils": "^0.2.5"
"loader-utils": "^1.0.2"
}
}
Loading