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

@aws-sdk/types 3.10.0 broke Typescript 3 compatibility #2217

Closed
legraphista opened this issue Apr 7, 2021 · 4 comments · Fixed by #2218
Closed

@aws-sdk/types 3.10.0 broke Typescript 3 compatibility #2217

legraphista opened this issue Apr 7, 2021 · 4 comments · Fixed by #2218
Labels
bug This issue is a bug.

Comments

@legraphista
Copy link

Describe the bug

This line is not backward compatible to Typescript V3

export type UserAgentPair = [name: string, version?: string];

Your environment

SDK version number

@aws-sdk/[email protected]

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

Node.js

Details of the browser/Node.js/ReactNative version

node -v: v12.22.1
tsc -v: Version 3.9.9

Steps to reproduce

package.json

{
  "dependencies": {
    "@aws-sdk/types": "^3.10.0"
  }
}

test.ts

import '@aws-sdk/types'

terminal

yarn && tsc --lib test.ts

Observed behavior

node_modules/@aws-sdk/types/dist/types/util.d.ts:92:42 - error TS1005: ',' expected.

92 export declare type UserAgentPair = [name: string, version?: string];
                                            ~

node_modules/@aws-sdk/types/dist/types/util.d.ts:92:60 - error TS1005: ',' expected.

92 export declare type UserAgentPair = [name: string, version?: string];
                                                              ~

Expected behavior

test.ts should compile with no errors

Additional context

@aws-sdk/types v3.6.1 had no issues compiling in TS3

@legraphista legraphista added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 7, 2021
@trivikr trivikr removed the needs-triage This issue or PR still needs to be triaged. label Apr 7, 2021
@trivikr
Copy link
Member

trivikr commented Apr 7, 2021

This issue seems to be happening, as postbuild is run twice on packages/types

Verified as follows:

> yarn build
yarn run v1.22.5
$ yarn build:es && yarn build:cjs
$ tsc -p tsconfig.es.json
$ tsc -p tsconfig.cjs.json
$ downlevel-dts dist/types dist/types/ts3.4
Done in 9.06s.

> ls dist/types 
abort.d.ts  client.d.ts  command.d.ts  credentials.d.ts  crypto.d.ts  eventStream.d.ts  http.d.ts  index.d.ts  logger.d.ts  middleware.d.ts  pagination.d.ts  response.d.ts  serde.d.ts  signature.d.ts  transfer.d.ts  ts3.4  util.d.ts

> ls dist/types/ts3.4 
abort.d.ts  client.d.ts  command.d.ts  credentials.d.ts  crypto.d.ts  eventStream.d.ts  http.d.ts  index.d.ts  logger.d.ts  middleware.d.ts  pagination.d.ts  response.d.ts  serde.d.ts  signature.d.ts  transfer.d.ts  util.d.ts

> yarn build
yarn run v1.22.5

> yarn build:es && yarn build:cjs
$ tsc -p tsconfig.es.json
$ tsc -p tsconfig.cjs.json
$ downlevel-dts dist/types dist/types/ts3.4
Done in 4.33s.
dev-dsk-trivikr-2b-b5d0913a: types> ls dist/types/ts3.4
abort.d.ts  client.d.ts  command.d.ts  credentials.d.ts  crypto.d.ts  eventStream.d.ts  http.d.ts  index.d.ts  logger.d.ts  middleware.d.ts  pagination.d.ts  response.d.ts  serde.d.ts  signature.d.ts  transfer.d.ts  ts3.4  util.d.ts

This could be happening as packages/types is built twice, once as part of crypto-dependencies and second time as part of lerna run build.

"build:all": "yarn build:crypto-dependencies && lerna run build",

@trivikr
Copy link
Member

trivikr commented Apr 7, 2021

A probable fix would be to move downlevel-dts from postbuild to prepublishOnly for the following reasons:

A GitHub Action needs to be written to regularly run prepublish to ensure types are backward compatible.

@trivikr
Copy link
Member

trivikr commented Apr 7, 2021

Metrics:

build time with downlevel-dts in postbuild
$ git clean -dfx && yarn

$ time yarn build:all
...
Done in 850.77s.
yarn build:all  12588.02s user 307.90s system 1515% cpu 14:10.90 total
build time with downlevel-dts in prepublishOnly
$ git clean -dfx && yarn

$ time yarn build:all
...
Done in 736.90s.
yarn build:all  10956.08s user 248.51s system 1520% cpu 12:17.03 total

The SDK maintainers can explicitly run prepublishOnly if they want to test downlevel-dts

$ ./node_modules/.bin/lerna run prepublishOnly

@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 Apr 23, 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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants