Skip to content
This repository has been archived by the owner on Jun 11, 2020. It is now read-only.

mirroring commit from mu; don't ship production npm deps #18

Merged
merged 1 commit into from Dec 6, 2016

Conversation

earnubs
Copy link
Contributor

@earnubs earnubs commented Nov 30, 2016

This will delete devDeps from node_modules, reducing churn in staging and production branches and preventing shipping unused code in deployments.

@@ -62,6 +62,7 @@ $(DIST):
mkdir -p $(DISTDIR)
npm install || (cat npm-debug.log && exit 1)
npm run build
npm install --production || (cat npm-debug.log && exit 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why, but it seems it doesn't work here - meaning it doesn't remove dev dependencies.

I checked it out in MU and it seems to work there (calling npm install followed by npm install --production removes unnecessary dependencies). But in here it seems to do nothing.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I think I found why it's not working. We lack npm-shrinkwrap.json.

It seems that only with npm-shrinkwrap.json being present npm install followed by npm install --production actually removes dev dependencies.

@bartaz
Copy link
Contributor

bartaz commented Dec 1, 2016

Because of https://github.com/canonical-ols/javan-rhino/issues/217 this won't work.

We need to move some needed devDependencies to prod dependencies or make sure they are required only in dev environment.

@bartaz
Copy link
Contributor

bartaz commented Dec 1, 2016

We should first separate dev and prod servers and fix the dependencies as in https://github.com/canonical-ols/javan-rhino/pull/226

Copy link
Contributor

@bartaz bartaz left a comment

Choose a reason for hiding this comment

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

Should work now as we have #22 merged.

@earnubs earnubs merged commit 1b7ac14 into master Dec 6, 2016
@earnubs earnubs deleted the npm-production-deps branch December 6, 2016 10:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants