Keystone-classic: /admin/public/js/packages.js is checked in to git
There's an 840k file at /admin/public/js/packages.js containing compiled code. This file shouldn't be checked in to git, but generated by a tool.
josephg
All 7 comments
Sure, I agree. But at the moment, we can't use prepublish or postinstall hooks to do it, without breaking support for git-based installs. Which are critical to the development process at the moment. I've been checking in /admin/public/js/packages.js as a workaround, because it's least likely to break for users.
For context see npm/npm#3055 and npm/npm#3217. When 3055 lands we can fix this, or we can add more cruft to the codebase in the meantime to work around it.
https://www.npmjs.com/package/postinstall-build also exists but I'm not sure how stable it is across different versions of npm / node and different operating systems, so haven't been inclined to spend time finding out (potentially the hard way). My radar could be off but it feels hacky and potentially dangerous compared to the current process which adds unfortunate weight to the git repo but is otherwise totally solid.
JedWatson
on 1 Aug 2016
We need it for Heroku, don't ignore it. :)
terenzeyuen
on 2 Aug 2016
@terenzeyuen PRs with fixes, perhaps based on https://www.npmjs.com/package/postinstall-build, very much appreciated :)
wmertens
on 3 Aug 2016
@terenzeyuen can you clarify what you said? I took that to mean "I agree with Jed, don't mess with it right now" and I think @wmertens interpreted you differently.
If somebody wants to pick this up, I think if we do we're better off building the bundle when keystone starts, rather than as a "post install" hook. We can write it out somewhere based on a hash of Keystone's package.json, so it only gets built on first boot after install or in dev if you change something.
JedWatson
on 3 Aug 2016
hey @JedWatson, yes – I agree with not messing with it right now, mainly for git-based installations.
terenzeyuen
on 3 Aug 2016
@JedWatson the hash would need to come from the git commit, no? Also, not a big fan of building everywhere when only building for git would be sufficient. That can be achieved by storing under a hash and pre-building that hash for npm deploy.
wmertens
on 3 Aug 2016
Can this be closed?
mcclowes
on 14 Sep 2018
Related issues
ttsirkia
·
4Comments
zhdan88vadim
·
5Comments
kamontat
·
5Comments
koenoe
·
4Comments
joernroeder
·
5Comments
Most helpful comment
Sure, I agree. But at the moment, we can't use
prepublishorpostinstallhooks to do it, without breaking support for git-based installs. Which are critical to the development process at the moment. I've been checking in/admin/public/js/packages.jsas a workaround, because it's least likely to break for users.For context see npm/npm#3055 and npm/npm#3217. When 3055 lands we can fix this, or we can add more cruft to the codebase in the meantime to work around it.
https://www.npmjs.com/package/postinstall-build also exists but I'm not sure how stable it is across different versions of npm / node and different operating systems, so haven't been inclined to spend time finding out (potentially the hard way). My radar could be off but it feels hacky and potentially dangerous compared to the current process which adds unfortunate weight to the git repo but is otherwise totally solid.