Closed MikeKovarik closed 5 years ago
First of all: I am so sorry for never responding to this PR. The ideas you're outlining are great and worth consideration.
Generally speaking, if you're still interested, I'd merge this PR if we can ensure that the full end-to-end experience still works. I'd put it behind a major new release (ensuring that nobody gets all too surprised) and move on from there.
Hello,
Idea of this PR originated in issue #97 where I discussed how I'm using this module to not only build the appx from scratch but to continualy building appx bundles from existing pre-appx folder. Once my custom build script grew larger I decided to work it into a fork and use this PR as a discussion opener
I was originally building my app using the Desktop App Converter but quickly discovered that typing out those complicated commands is annoying whereas this module has some nice meticulously crafted functions that do just that. Though they are all hooked into the default pipeline. All that the DAC and this module does to the user's app is copying it elsewhere, adding
AppxManifest
andAssets
folder before making the actuall appx bundle. This copying part can be however skipped (and theconvert()
,manifest()
functions in the pipeline) if manifest and assets already exist in the app's folder and I've been using only themakeappx()
andsign()
to do the incrementally build my app with success. With added function that parses data from the manifest and increments version number on each build.And this is what I'm proposing with this PR. Alternative build process that uses pre existing app folder and manifest and can be called from the index.js instead of
convertToStore()
. It speeds up the process a lot by not having to copy anything and does not require to deal with version numbers.I tried not to change much but I did change arguments of
makeCert()
. It only accepts program now as its only argument and the publisher name and folder is now created inside the function instead of in thesetup()
. I also added a few more file or folder path properties to the program object likeprogram.appx
pointing to the place where the appx is expected to be created.program.manifest
now always includes path where the manifest is expected to be and additional fs check has to be done to verify actual existence of the file and some problems could arise if some code expects existence of the file solely by havingprogram.manifest
defined. In this casecopyTemplate()
might not work as expected. Though this one in particular is not expected to be ran in the incremental build mode (since the copying part is completely skipped) and I'm mentioning it as a kind of things to keep an eye on.Another thing worth mentioning is the new
sign.prepareCert()
and code around line 180 insetup()
that tries to look in the folder where the cert would have been previously generated instead of requiringprogram.devCert
to be defined.The whole
setup()
is changed a lot to allow for reading from manifest.Also some of the code is written using async/await syntax which is supported since Node 7.6 but I can rewrite it back to promises if backwards compatibility is required. Plus there are few unrelated changes, namely simplifying code of functions returning promises, changed capital A Assets to assets since to match with rest of the module, and a few more OCD driven touches :D
Like I said, this PR is not ready to be merged just yet. It is just the base implementation (working!) that however might've broken something elsewhere. There's a lot to discuss and I didn't want to push forward with fixing tests or writing new ones while there's a chance that this PR will get shot down :D. There are more ideas that can be added uppon this like syncing version numbers with package.json and using package.json in general to reduce the number of CLI arguments. Or gulp plugin.
But this is just the beginning.