-
-
Notifications
You must be signed in to change notification settings - Fork 54
Fix support for CRX uploads #111
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
Co-authored-by: fregante <[email protected]>
Co-authored-by: fregante <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
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.
@copilot
source/index.ts
Outdated
| const stats = await fs.promises.stat(streamOrPath); | ||
| readStream = stats.isFile() | ||
| ? fs.createReadStream(streamOrPath) | ||
| : await zipStreamFromDirectory(streamOrPath); |
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.
You don't need to change this block. Restore the readStream line as it was
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.
Restored the original readStream assignment pattern with the ternary operator and getStreamFromPath helper function. Commit: 94a3f53
Co-authored-by: fregante <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
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.
I expected X-Goog-Upload-File-Name to match the actual file name but the code seems to work even when it doesn’t. Confirmed working on my end.
|
Yeah I figured it doesn't matter what we pass, they only check the extension anyway. Thanks for checking! The only problem with this solution is that it won't work if you pass a CRX stream but only if you pass a path. |
|
I'll release this package as a patch update and then the CLI will receive a major prerelease that you can try. Maybe in a couple of hours |
Isn't it because My test codeI believe there are 12 possible cases. Cases
Right now #4 works. For #1, the #2 and #5 are invalid. If you're opted in you can't use other than CRX. They return I believe #3 is invalid because you can't stream folders. #6 doesn't work for me. I have folders in my extension and Since I've opted in, I can't/didn't test the rest. Right now I can't spend time to make another test extension. But I suggest you to do it and use proper CRX files for testing. I would try to help implementing it but I don't know Node.JS/NPM stuf and I don't want to spend time learning it because I won't be using it anywhere else in the near future. I also suggest you to test the effects of adding or not adding |
|
Yes? That's the point: if you pass a stream, the package doesn't know its name because streams don't have names. Folders aren't "streamed", they're zipped first. There are 3 cases:
There's nothing to do or fix here. As was clear from the beginning, I'm not going to implement crx creation, so 6 will never work. If you want crx uploads, you must pass a crx file path. |
_headersmethod to support upload-specific headersuploadExistingmethod to include the new headersSummary of Changes
Added support for Google upload protocol headers in the
uploadExistingmethod:New
_uploadHeadersmethod: Creates upload-specific headers that include:X-Goog-Upload-Protocol: rawX-Goog-Upload-File-Name: <filename>Simplified filename detection:
extension.crxextension.zipPreserved original code structure:
getStreamFromPathhelper functionreadStreamassignmentAdded comprehensive tests to verify:
All existing tests continue to pass, ensuring backward compatibility.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.