[eas-cli] add publish.ts 3/4 #147
Conversation
|
Size Change: +38.7 kB (0%) Total Size: 42.5 MB
|
| }; | ||
| }; | ||
|
|
||
| export function guessContentTypeFromExtension(ext?: string): string { |
There was a problem hiding this comment.
The content type is important for asset storage. In the future we may ask the user to specify the contentType if we don't recognize it. For now we default to 'application/octet-stream'
| }; | ||
| } | ||
|
|
||
| export function buildUpdateInfoGroup(assets: CollectedAssets) { |
There was a problem hiding this comment.
Good point, I made this PR to add noImplicitReturns to the tsconfig to help catch this in the future: #166
| @@ -57,26 +57,30 @@ async function obtainS3PresignedPostAsync( | |||
| export async function uploadWithPresignedPostAsync( | |||
| stream: Readable | Buffer, | |||
| presignedPost: PresignedPost, | |||
| handleProgressEvent: ProgressHandler | |||
| ) { | |||
| handleProgressEvent?: ProgressHandler | |||
There was a problem hiding this comment.
Allow handleProgressEvent to be optional. Uploading 100's of assets would look chaotic.
| @@ -1,17 +1,13 @@ | |||
| import gql from 'graphql-tag'; | |||
|
|
|||
| import { graphqlClient, withErrorHandlingAsync } from '../client'; | |||
| import { AssetMetadataResult } from '../generated'; | |||
There was a problem hiding this comment.
Changed to generated type.
| handleProgressEvent({ isComplete: true }); | ||
| return String(response.headers.location); | ||
| } catch (error) { | ||
| handleProgressEvent({ isComplete: true, error }); |
There was a problem hiding this comment.
Not sure if we need these synthetic events after success/error because the caller can as easily trigger the necessary actions after the promise has resolved. Removing them (and additionally using the Progress event from got without wrapping it) would also greatly simplify this code:
const uploadPromise = got.post(presignedPost.url, { body: form, headers: { ...formHeaders } });
if (handleProgressEvent) {
uploadPromise.on('uploadProgress', handleProgressEvent);
}
const response = await uploadPromise;
return String(response.headers.location);There was a problem hiding this comment.
This is much more concise, but looking through the code for a progress handler, it seems to expect an isComplete argument.
See this recent PR in particular: https://github.com/expo/eas-cli/pull/153/files#diff-54650d756e5f6628f0b3c7c2564229e97085d05dacc0fac8263e0fcb9647da47R70
Simplifying this code seems desirable, but perhaps worth consulting @EvanBacon first and doing in a separate PR.
There was a problem hiding this comment.
I think it might be still best to keep this function as simple as possible and then implement the progress handler interface in publish.ts, because it will need to keep track of all the uploads in order to show a combined progress bar. And I think the progress bar can be added in another PR.
Why
This is the third of 4 PR's to add
eas release:publish.It adds all of the helper functions used by the command.
It is the most complex of the 4. It maybe worth cross referencing the final #146 to see how these functions are used.
How
Test Plan
yarn test
and also
tested in #146