-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add header input type to support multi headers #8
Conversation
ts-force/src/rest/restHeader.ts
Outdated
'If-Unmodified-Since'?: Date; | ||
'If-Match'?: string[]; | ||
'If-None-Match'?: string[]; | ||
'Generic-Header'?: GenericHeader; |
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.
Is Generic-Header
just allowing an arbitrary header key/value?
If so, it seems like you would want to potentially support multiple values. It might also be easier to just allow a Record<string, string>
.
additionalHeaders?: Record<string, string>;
ts-force/src/rest/restObject.ts
Outdated
ConditionalRequestHeaders.includes(key as keyof RequestHeadersInput) | ||
); | ||
if (containsConditionalHeader && (response.status === 304 || response.status === 412)) { | ||
throw new ConditionalError('Conditions not met'); |
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.
Might want to include the header keys in the exception
ts-force/src/rest/restHeader.ts
Outdated
|
||
export const ConditionalRequestHeaders: (keyof RequestHeadersInput)[] = [ | ||
'If-Match', | ||
'If-None-Match', |
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've never actually used these etag
caching headers with Salesforce... Reviewing the docs, I see this statement:
An optional header specifying a comma-delimited list of one or more ETags. This only has an effect when used with Account objects. The request is only processed if the Account’s ETag does not match one of the ETags in the list.
Does it really only work with Account
?
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.
From the document here, Etag is used for http caching. I guess that’s the reason why it’s said only work with Account. I test with a a case project, and If-Match
and If-None-Match
can be added on get request but not on patch request.
@@ -6,8 +6,6 @@ import { getCalendarDate } from '../../utils/calendarDate'; | |||
import { Account, Contact, User } from '../assets/sobs'; |
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 would recommend undoing the formatting changes
ts-force-gen/package.json
Outdated
@@ -1,6 +1,6 @@ | |||
{ | |||
"name": "@vacasaoss/ts-force-gen", | |||
"version": "3.4.4", | |||
"version": "3.4.5", |
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.
Is this a breaking change? Should we increment the major version?
Currently, ts-force doesn't support adding headers. Based on this, we add some headers to send in patch requests. The below are the headers we intend to add.