-
Notifications
You must be signed in to change notification settings - Fork 70
feat: add support for CRXv3 #98
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
Conversation
Updated package with latest master. |
@oncletom any chance this (with or without additional changes), or something like this could be merged :)? |
Any update on this, now that Chrome 73 has landed in stable and requires the CRX3 format? |
Bump. Latest chrome does indeed require v3. Any update on this? |
Also looking for an update on this, I use this package with my automated releases so this PR is pretty critical for me! |
This tool is useless without v3 support. What's blocking it? I'd offer my help if necessary. |
In meantime i created https://github.com/ahwayakchih/crx3, maybe it can be useful for others too while we wait for update to this module. @oncletom please check it - hopefully changes can be integrated into this module in the future. I wrote it so all data is streamed and whole ZIP file does not have to be kept in RAM. |
@ansoni-san Thanks for the "useless". I'm moving between places and working to pay my bills and eventually get some free time to work on unpaid projects like this one. That's the blocker. Do you feel better with this answer? I'm grateful for @ahwayakchih code proposal and for you all to discuss the outcome of this missing feature. I'm sorry for not responding and keeping all of you in the dark. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looks good to me. I'm assuming the logic borrowed from Chromium is fine. ¯\_(ツ)_/¯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, thank you so much for this piece of work and for being patient 🙂
Commander module: - does not allow setting defaults, - does not call custom parsing function on optional parameters, - does not seem to provide a way to set values manually (like: `program.myOptionValue = 3`). Which means we don't have a clean way to be sure that `program.crxVersion` defaults to 3 :(.
Missed it in thom4parisot#98, sorry.
This adds
--format-version
option that defaults to2
, but can be set to3
.Current (72.0.3626.109 here) versions of Chromium generate CRX files in v3 format. V2 is still supported, but who knows for how long.
Ideally, v2 code would also be extracted to separate
crx2.js
file, but i did not want to mess too much around original code. Just let me know if you think that's a good idea (it would break API, becausegenerateSignature
would become private/hidden from outside, as in v3).I also added
--verbose
option locally (for bothkeygen
andbuild
commands), but since your ESLint setup forbids usingconsole.log
, i removed it from this pull request.This adds dependency on
pbf
module. I added pre-generatedcrx3.js.pb
(changed extension name to hide it from linter and tests ;). If you would like to import originalcrx3.proto
file instead, and automatically rebuildcrx3.js.pb
whenever proto file changes, let me know, but that would probably require some kinda of build task to add to this project).I did not try to use this in a browser. I only tested generating CRX packages using node.js v11.10.0.