-
Notifications
You must be signed in to change notification settings - Fork 181
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
chore(server): easy js to ts migrations #2 - objectStorage.ts #3396
Conversation
} catch (err) { | ||
if (err instanceof S3ServiceException && err.Code === 'NoSuchKey') | ||
if (err instanceof S3ServiceException && get(err, 'Code') === 'NoSuchKey') |
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.
get(err, Code)
no such prop according to TS, hence the dynamic access
@@ -51,32 +61,45 @@ const getObjectStorage = () => ({ | |||
createBucket: createS3Bucket() | |||
}) | |||
|
|||
const sendCommand = async (command) => { | |||
const sendCommand = async <CommandOutput>( |
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.
ok, nice use of generics. I think this is where I got stuck!
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 notice that when sendCommand
is called we don't specify what CommandOutput is. Is it inferred from the usage?
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.
yes
return await client.send(command(Bucket)) | ||
const ret = (await client.send( | ||
command(Bucket) as Command<any, any, any, any, any> | ||
)) as CommandOutput |
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'm not a fan of as CommandOutput
. Is there a safer way of doing this?
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.
It's already as CommandOutput
in practice, the type of CommandOutput is auto inferred from the actual command you pass in. The only reason I need to do this is because of all of the abstraction and generics behind the scene which break something - I dunno, it wasn't obvious to me how to avoid it.
Description & motivation
Changes:
To-do before merge:
Screenshots:
Validation of changes:
Checklist:
References