-
Notifications
You must be signed in to change notification settings - Fork 1
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
Install devDependencies by default #519
Changes from 7 commits
18e3b01
1fddd31
30f5499
6fbbaaf
92d5b9f
cae423a
751bf1b
74c3e87
4c44116
bf4c75c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -211,6 +211,17 @@ cache_build() { | |
header "Caching build" | ||
cache_build | output "$LOG_FILE" | ||
|
||
prune_devdependencies() { | ||
if $YARN; then | ||
yarn_prune_devdependencies "$BUILD_DIR" | ||
else | ||
npm_prune_devdependencies "$BUILD_DIR" | ||
fi | ||
} | ||
|
||
header "Pruning devDependencies" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the terminology that npm uses, but I don't love it. Is there a clearer way to get across "we're removing the modules defined in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's the command in both |
||
prune_devdependencies | output "$LOG_FILE" | ||
|
||
summarize_build() { | ||
if $NODE_VERBOSE; then | ||
list_dependencies "$BUILD_DIR" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,14 +87,34 @@ log_build_scripts() { | |
|
||
yarn_node_modules() { | ||
local build_dir=${1:-} | ||
local production=${YARN_PRODUCTION:-false} | ||
|
||
echo "Installing node modules (yarn.lock)" | ||
cd "$build_dir" | ||
yarn install --frozen-lockfile --ignore-engines 2>&1 | ||
yarn install --production=$production --frozen-lockfile --ignore-engines 2>&1 | ||
} | ||
|
||
yarn_prune_devdependencies() { | ||
local build_dir=${1:-} | ||
|
||
if [ $NODE_ENV == "test" ]; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will be caught by the |
||
echo "Skipping because NODE_ENV is 'test'" | ||
return 0 | ||
elif [ $NODE_ENV != "production" ]; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be worth quoting the eg: $ NODE_ENV="foo bar"; if [ $NODE_ENV != "production" ]; then echo "not production"; else echo "production"; fi
bash: [: too many arguments
production Fwiw shellcheck is able to find issues like this (the Python buildpack runs it on Travis): In lib/dependencies.sh line 103:
elif [ $NODE_ENV != "production" ]; then
^-- SC2086: Double quote to prevent globbing and word splitting. (https://github.com/koalaman/shellcheck/wiki/SC2086) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cough shellcheck is the best |
||
echo "Skipping because NODE_ENV is not 'production'" | ||
return 0 | ||
elif [ -n "$YARN_PRODUCTION" ] && [ "$YARN_PRODUCTION" != "true" ]; then | ||
echo "Skipping because YARN_PRODUCTION is not 'true'" | ||
return 0 | ||
else | ||
cd "$build_dir" | ||
yarn install --frozen-lockfile --ignore-engines --ignore-scripts --prefer-offline 2>&1 | ||
fi | ||
} | ||
|
||
npm_node_modules() { | ||
local build_dir=${1:-} | ||
local production=${NPM_CONFIG_PRODUCTION:-false} | ||
|
||
if [ -e $build_dir/package.json ]; then | ||
cd $build_dir | ||
|
@@ -106,14 +126,15 @@ npm_node_modules() { | |
else | ||
echo "Installing node modules (package.json)" | ||
fi | ||
npm install --unsafe-perm --userconfig $build_dir/.npmrc 2>&1 | ||
npm install --production=$production --unsafe-perm --userconfig $build_dir/.npmrc 2>&1 | ||
else | ||
echo "Skipping (no package.json)" | ||
fi | ||
} | ||
|
||
npm_rebuild() { | ||
local build_dir=${1:-} | ||
local production=${NPM_CONFIG_PRODUCTION:-false} | ||
|
||
if [ -e $build_dir/package.json ]; then | ||
cd $build_dir | ||
|
@@ -124,8 +145,32 @@ npm_rebuild() { | |
else | ||
echo "Installing any new modules (package.json)" | ||
fi | ||
npm install --unsafe-perm --userconfig $build_dir/.npmrc 2>&1 | ||
npm install --production=$production --unsafe-perm --userconfig $build_dir/.npmrc 2>&1 | ||
else | ||
echo "Skipping (no package.json)" | ||
fi | ||
} | ||
|
||
npm_prune_devdependencies() { | ||
local build_dir=${1:-} | ||
local npm_version=$(npm --version) | ||
|
||
if [ $NODE_ENV == "test" ]; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as the yarn comment above, but wanted to point it out for completeness sake. |
||
echo "Skipping because NODE_ENV is 'test'" | ||
return 0 | ||
elif [ $NODE_ENV != "production" ]; then | ||
echo "Skipping because NODE_ENV is not 'production'" | ||
return 0 | ||
elif [ -n "$NPM_CONFIG_PRODUCTION" ] && [ "$NPM_CONFIG_PRODUCTION" != "true" ]; then | ||
echo "Skipping because NPM_CONFIG_PRODUCTION is not 'true'" | ||
return 0 | ||
elif [ $npm_version == "5.3.0" ]; then | ||
mcount "skip-prune-issue-npm-5.3.0" | ||
echo "Skipping because npm 5.3.0 fails when running 'npm prune' due to a known issue" | ||
echo "https://github.com/npm/npm/issues/17781" | ||
return 0 | ||
else | ||
cd "$build_dir" | ||
npm prune --userconfig $build_dir/.npmrc 2>&1 | ||
fi | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -122,8 +122,7 @@ fail_yarn_outdated() { | |
local log_file="$1" | ||
local yarn_engine=$(read_json "$BUILD_DIR/package.json" ".engines.yarn") | ||
|
||
if grep -qi 'error: unknown option .--frozen-lockfile' "$log_file"; then | ||
echo "ran" | ||
if grep -qi 'error .install. has been replaced with .add. to add new dependencies' "$log_file"; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is changed because in these earlier versions adding the |
||
mcount "failures.outdated-yarn" | ||
echo "" | ||
warn "Outdated Yarn version: $yarn_engine | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
{ | ||
"name": "node-buildpack-test-app", | ||
"version": "0.0.1", | ||
"description": "node buildpack integration test app", | ||
"repository": { | ||
"type": "git", | ||
"url": "http://github.com/example/example.git" | ||
}, | ||
"dependencies": { | ||
"hashish": "*" | ||
}, | ||
"license": "MIT", | ||
"engines": { | ||
"yarn": "1.4.0" | ||
}, | ||
"devDependencies": { | ||
"lodash": "^2.4.1" | ||
}, | ||
"scripts": { | ||
"test": "ls node_modules" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. | ||
# yarn lockfile v1 | ||
|
||
|
||
hashish@*: | ||
version "0.0.4" | ||
resolved "https://registry.yarnpkg.com/hashish/-/hashish-0.0.4.tgz#6d60bc6ffaf711b6afd60e426d077988014e6554" | ||
dependencies: | ||
traverse ">=0.2.4" | ||
|
||
lodash@^2.4.1: | ||
version "2.4.2" | ||
resolved "https://registry.yarnpkg.com/lodash/-/lodash-2.4.2.tgz#fadd834b9683073da179b3eae6d9c0d15053f73e" | ||
|
||
traverse@>=0.2.4: | ||
version "0.6.6" | ||
resolved "https://registry.yarnpkg.com/traverse/-/traverse-0.6.6.tgz#cbdf560fd7b9af632502fed40f918c157ea97137" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
A fake README, to keep npm from polluting stderr. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
A fake README, to keep npm from polluting stderr. |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changes a lot of the fixture files for tests which can be mostly ignored. The important changes are found in:
bin/compile
lib/dependencies.sh
lib/environment.sh
lib/failure.sh
With tests changes in:
test/run