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

Generate lockfile at build time. #668

Open
nathanhammond opened this issue Mar 12, 2019 · 4 comments · May be fixed by #671
Open

Generate lockfile at build time. #668

nathanhammond opened this issue Mar 12, 2019 · 4 comments · May be fixed by #671

Comments

@nathanhammond
Copy link
Contributor

When creating a production build of an application, the build should be repeatable. In order for a build to be repeatable, any dependencies and transitives should be locked.

This is not presently possible using ember-cli-fastboot as it only spits out a package.json file. We should extend it to also emit a package-lock.json or yarn.lock.

@nathanhammond
Copy link
Contributor Author

So far as I can tell the only way to accomplish this task consistently for npm is to:

  • Set the npm cache to an empty directory that is under our control.
  • Run npm install for the project.
  • Run ember build to spit out the new package.json.
  • Run npm install --offline against the generated package.json using our cache.
  • Capture the package-lock.json that is created for that build.

Doing this would sidestep the issue for #669.

@rwjblue
Copy link
Member

rwjblue commented Mar 13, 2019

I agree that we should have a way to make the fastboot assets include a lock file, however I'm not sure that I agree with the assumptions in your list. For example:

  • I don't think it makes much sense to do this during development builds
  • Its not clear to me why you care about mucking with the npm cache, what does this gain?

My gut feeling is that this should be the responsibility of the thing preparing a specific build for actual use. Maybe that means ember b -prod, but it could also easily mean "on extract for deployment" (ala ember-cli-deploy's prepare hook).

@nathanhammond
Copy link
Contributor Author

nathanhammond commented Mar 13, 2019

I don't think it makes much sense to do this during development builds

Agreed, the very first line of my issue is "When creating a production build". 😜 The question of "who should be responsible" is kind of nebulous (it's code, we can make any entity responsible), but I tend toward "whatever component is producing the package.json".

I actually did create a version that works with a custom build task in Ember CLI but I no longer believe that is the right place to do it. Going and looking for a possibly-emitted package.json and then generating a lockfile doesn't really seem like it should be the responsibility of Ember CLI. To my knowledge, FastBoot is the only addon out there which spits out a package.json so it makes sense to me that we make it responsible for locking those dependencies.

Its not clear to me why you care about mucking with the npm cache, what does this gain?

This was me exploring ways to create a reproducible build from only the packages that were pulled down for the asset already. For correctness, the package.json can't be allowed to resolve to something newer from the network or the global cache because of unpinned versions.

but it could also easily mean "on extract for deployment" (ala ember-cli-deploy's prepare hook).

This is too late and doesn't actually solve the problem. The produced artifact needs to bundle the lockfile. (This is also the asset you would build as a standalone thing before passing it off to N testing servers.) The prepare hook is where you would run install against the lockfile, not where you generate the lockfile. (In this scenario the lockfile is now disconnected from the thing that built it. The thing that built it is the Ember Application which actually knows the versions that it needs.)


Also, in terms of motivation, this isn't theoretical. I've encountered an issue in the past where I was unable to deploy a production hotfix (!) because FastBoot dependencies weren't pinned and we were picking up a new and incompatible version of a package.

@nathanhammond nathanhammond linked a pull request Mar 13, 2019 that will close this issue
@nathanhammond
Copy link
Contributor Author

Alternative paths explored that sidestep the two installation passes:

  • Parsing content out of package-lock.json: this ends up being a bad reimplementation of npm install and, by virtue of being custom, isn't guaranteed to be correct (though it would presumably at least be consistently broken).
  • Pinning the top level version directly (a la getDependencyVersion should pin version to the installed version. #669). This does not solve for transitive dependencies.
  • Simply copying and bundling the node_modules folder from the application. Since the dependency hierarchy changes, this is not guaranteed to actually create a working resolution for the application.
  • yarn why a billion times is not a performance win.

... all this to say that the unfortunate double build is likely the best of the options. 😒

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 a pull request may close this issue.

2 participants