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

Use package-lock.json to install packages if there is one available. #246

Merged
merged 4 commits into from Oct 3, 2017

Conversation

HyperBrain
Copy link
Member

@HyperBrain HyperBrain commented Oct 2, 2017

What did you implement:

Closes #245

How did you implement it:

If a package-lock.json is found besides the package.json of the project, it is copied to the packaged dependencies folder, so that the plugin now installs the locked versions (for components that are present in the lock file).
The package.json used for packaging is now pre-assembled because only an unqualified npm install uses an existing lock file.

How can we verify it:

Use a project with locked dependencies, package and check the created zip file.

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

return new BbPromise((resolve, reject) => {
// (1.a.2) Copy package-lock.json if it exists, to prevent unwanted upgrades
const packageLockPath = path.join(path.dirname(packageJsonPath), 'package-lock.json');
return BbPromise.fromCallback(cb => fse.pathExists(packageLockPath, cb))
Copy link

Choose a reason for hiding this comment

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

Why not use the sync versions if you're using it in a synchronous way?

Copy link
Member Author

@HyperBrain HyperBrain Oct 3, 2017

Choose a reason for hiding this comment

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

Because it will stall the Node event loop (even if something else might be running in the background - outside of the function) - we still run in the Serverless context and should not affect our host in any way ;-) This makes sure that we have a "thread/task" switch.

Copy link

Choose a reason for hiding this comment

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

Tell that to the Serverless guys using synchronous functions whenever humanly possible 🤣

Copy link
Member Author

Choose a reason for hiding this comment

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

Haha... In the meantime they even turned some of the stuff asynchronously, like file copy.

@HyperBrain HyperBrain merged commit 85b60bf into master Oct 3, 2017
@HyperBrain HyperBrain deleted the use-package-lock branch October 3, 2017 13:36
splitModule[0] = '@' + splitModule[0];
}
const moduleVersion = _.join(_.tail(splitModule), '@');
_.set(compositePackage, `dependencies.${_.first(splitModule)}`, moduleVersion);
Copy link
Member Author

Choose a reason for hiding this comment

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

@silver2k This is the issue. The assembly of the dependency name breaks here if the dependency includes a ..

It is handled as if it was a nested property. I will provide a fix asap and release 3.1.2.

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

Successfully merging this pull request may close these issues.

package-lock.json is not respected when bundling
2 participants