Skip to content
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

Typescript error when using lib-storage upload #2085

Closed
inian opened this issue Feb 25, 2021 · 3 comments
Closed

Typescript error when using lib-storage upload #2085

inian opened this issue Feb 25, 2021 · 3 comments
Labels
bug This issue is a bug. guidance General information and guidance, answers to FAQs, or recommended best practices/resources.

Comments

@inian
Copy link

inian commented Feb 25, 2021

Describe the bug

const data = await request.file()

const paralellUploads3 = new Upload({
      client,
      params: {
        Bucket,
        Key,
        Body: data.file
      },
    })

In the above code example, data.file is of type NodeJS.ReadableStream.

When I pass that to Upload, it errors out with the following error

error TS2322: Type 'ReadableStream' is not assignable to type 'string | Uint8Array | Blob | ReadableStream<any> | Buffer | Readable | undefined'.
  Type 'ReadableStream' is missing the following properties from type 'Readable': readableEncoding, readableEnded, readableFlowing, readableHighWaterMark, and 7 more.

193         Body: data.file,
            ~~~~

  node_modules/@aws-sdk/client-s3/types/commands/PutObjectCommand.d.ts:7:5
    7     Body?: PutObjectRequest["Body"] | string | Uint8Array | Buffer;
          ~~~~
    The expected type comes from property 'Body' which is declared here on type 'PutObjectCommandInput'

I think this is happening because it is taking in the type definition of WHATWG streams and trying to match with NodeJS.Readable stream.

I had to include DOM types in my tsconfig lib because of this. If I remove "DOM" types from tsconfig, I am getting other errors like this

node_modules/@aws-sdk/client-s3/types/models/models_0.d.ts:5440:23 - error TS2304: Cannot find name 'ReadableStream'.

5440     Body?: Readable | ReadableStream | Blob;
                           ~~~~~~~~~~~~~~

node_modules/@aws-sdk/client-s3/types/models/models_0.d.ts:5440:40 - error TS2304: Cannot find name 'Blob'.

5440     Body?: Readable | ReadableStream | Blob;
                                            ~~~~

How do I force typescript to assume it is a NodeJS.Readable stream here and not a WHATWG stream?

I was able to fix the error by changing the type definition here to NodeJS.ReadableStream instead of ReadableStream and adding "DOM" to my tsconfig lib.

Your environment

SDK version number

@aws-sdk/lib-storage@3.6.1

Is the issue in the browser/Node.js/ReactNative?

Node.js

Details of the browser/Node.js/ReactNative version

Node v12.20.2

@inian inian added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 25, 2021
@alexforsyth
Copy link
Contributor

@inian this doesnt seem like a bug with the SDK, the idea is to bring in the same types that S3 says it can take. To expand those types we'd need to expand the putObject types and that carries a bunch of additional work (mostly around testing) to make sure everything works as intended.

Also, sorry about having to include DOM types in the tsconfig! We made this choice internally, i know its not ideal and I'm happy to revisit if its a huge issue with the community. I see you linked #1688 where I touched on this.

Since this is just a type error, to get rid of the typescript error in your code immediately you can do something like:

const data = await request.file()

const paralellUploads3 = new Upload({
     client,
     params: {
       Bucket,
       Key,
       Body: (data.file as any) or (data.file as Readable)
     },
   })

I know its a bit ugly, but we probably wont be able to prioritize adding NodeJS.ReadableStream as a valid data input type.

@alexforsyth alexforsyth added guidance General information and guidance, answers to FAQs, or recommended best practices/resources. and removed needs-triage This issue or PR still needs to be triaged. labels Feb 26, 2021
@G-Rath
Copy link

G-Rath commented Mar 12, 2021

@inian you can remove dom lib by declaring the missing interfaces like so:

// @aws-sdk/client-s3 references these DOM lib interfaces,
// so we need them to exist to compile without having DOM.
declare global {
  /* eslint-disable @typescript-eslint/no-empty-interface */
  interface Blob {}
  interface ReadableStream {}
  /* eslint-enable @typescript-eslint/no-empty-interface */
}

This will mean you don't need to do any casting nor have to risk false negatives due to interfaces and globals defined in the DOM lib :)

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug. guidance General information and guidance, answers to FAQs, or recommended best practices/resources.
Projects
None yet
Development

No branches or pull requests

3 participants