Sdk improvements #56
Sdk improvements #56
Conversation
|
This pull request introduces 1 alert when merging e11e356 into 05f6002 - view on LGTM.com new alerts:
|
imagekitio
left a comment
imagekitio
left a comment
There was a problem hiding this comment.
Test cases are failing.
src/utils/request.ts
Outdated
| } | ||
|
|
||
| export const request = (uploadFileXHR: XMLHttpRequest,formData: FormData, options: ImageKitOptions & { authenticationEndpoint: string }, callback?: (err: Error | null, response: UploadResponse | null) => void) => { | ||
| var signatureXHR = new XMLHttpRequest(); |
There was a problem hiding this comment.
Why do we need to create xhr for signature here and change generateSignatureToken at all?
|
This pull request introduces 1 alert when merging f9c18d6 into 05f6002 - view on LGTM.com new alerts:
|
src/upload/index.ts
Outdated
| else if(Array.isArray(param)) { | ||
| formData.append(i, JSON.stringify(param)); | ||
| } | ||
| else if(typeof param === "object" && !(param instanceof File || param instanceof Blob)) { |
There was a problem hiding this comment.
Node.js sdk is doing it in a better way. You can replace this whole thing.
https://github.com/imagekit-developer/imagekit-nodejs/blob/master/libs/upload/index.ts#L50
Codecov Report
@@ Coverage Diff @@
## dev #56 +/- ##
=======================================
Coverage 94.53% 94.53%
=======================================
Files 9 9
Lines 183 183
Branches 41 41
=======================================
Hits 173 173
Misses 4 4
Partials 6 6 Continue to review full report at Codecov.
|
|
This pull request introduces 1 alert when merging 20a9602 into 05f6002 - view on LGTM.com new alerts:
|
imagekitio
left a comment
imagekitio
left a comment
There was a problem hiding this comment.
Readme.md needs to be updated.
src/index.ts
Outdated
|
|
||
| const promisify = function <T = void>(thisContext: ImageKit, fn: Function) { | ||
| return function (...args: any[]): Promise<T> | void { | ||
| console.log('arg: ', args); |
| expect(arg.get('isPrivateFile')).to.be.equal('true'); | ||
| expect(arg.get('publicKey')).to.be.equal('test_public_key'); | ||
| expect(arg.get('extensions')).to.be.equal(jsonStringifiedExtensions); | ||
| expect(arg.get('overwriteFile')).to.be.equal('false'); |
There was a problem hiding this comment.
Assertion for customMetadata is missing.
imagekitio
left a comment
imagekitio
left a comment
There was a problem hiding this comment.
Test cases for exposing xhr in callback is missing.
imagekitio
left a comment
imagekitio
left a comment
There was a problem hiding this comment.
Test cases for ResponseMetadata is missing.
|
This pull request introduces 1 alert when merging e74ca08 into 05f6002 - view on LGTM.com new alerts:
|
3 participants
No description provided.