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

cdk init : tsconfig has DOM as bundled library #28428

Open
niko-achilles opened this issue Dec 19, 2023 · 4 comments
Open

cdk init : tsconfig has DOM as bundled library #28428

niko-achilles opened this issue Dec 19, 2023 · 4 comments
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort p3 package/tools Related to AWS CDK Tools or CLI

Comments

@niko-achilles
Copy link

Describe the bug

When creating a cdk app by using the cli command: cdk init sample-app --language=typescript then a sample project is created with a tsconfig that looks as follows:

{
  "compilerOptions": {
    "target": "ES2020",
    "module": "commonjs",
    "lib": ["es2020", "DOM"],
   ...
  },
  ...
}

DOM as declaration for lib is not needed. CDK will not executed in the DOM , so the DOM libraries should not be used when creating cdk apps.

Furthermore the package.json has as dev. dependency the "@types/node": "20.10.4", which is correct and can support the developer to use the typescript types defined for node.

Expected Behavior

When creating a cdk app by using the cli command: cdk init sample-app --language=typescript then a sample project should be created with a tsconfig that does not have DOM as lib declaration as follows :

{
  "compilerOptions": {
    "target": "ES2020",
    "module": "commonjs",
    "lib": ["es2020"],
    "declaration": true,
    "strict": true,
    "noImplicitAny": true,
    "strictNullChecks": true,
    "noImplicitThis": true,
    "alwaysStrict": true,
    "noUnusedLocals": false,
    "noUnusedParameters": false,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": false,
    "inlineSourceMap": true,
    "inlineSources": true,
    "experimentalDecorators": true,
    "strictPropertyInitialization": false,
    "typeRoots": ["./node_modules/@types"]
  },
  "exclude": ["node_modules", "cdk.out"]
}

Current Behavior

When creating a cdk app by using the cli command: cdk init sample-app --language=typescript then a sample project is created with a tsconfig that looks as follows:

NOTE : DOM as declaration for lib is defined. CDK will not executed in the DOM , so the DOM libraries should not be used when creating cdk apps.

{
  "compilerOptions": {
    "target": "ES2020",
    "module": "commonjs",
    "lib": ["es2020", "DOM"],
    "declaration": true,
    "strict": true,
    "noImplicitAny": true,
    "strictNullChecks": true,
    "noImplicitThis": true,
    "alwaysStrict": true,
    "noUnusedLocals": false,
    "noUnusedParameters": false,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": false,
    "inlineSourceMap": true,
    "inlineSources": true,
    "experimentalDecorators": true,
    "strictPropertyInitialization": false,
    "typeRoots": ["./node_modules/@types"]
  },
  "exclude": ["node_modules", "cdk.out"]
}

Reproduction Steps

cdk init sample-app --language=typescript

Possible Solution

When creating a cdk app by using the cli command: cdk init sample-app --language=typescript then a sample project should be created with a tsconfig that does not have DOM as lib declaration as follows :

{
  "compilerOptions": {
    "target": "ES2020",
    "module": "commonjs",
    "lib": ["es2020"],
    "declaration": true,
    "strict": true,
    "noImplicitAny": true,
    "strictNullChecks": true,
    "noImplicitThis": true,
    "alwaysStrict": true,
    "noUnusedLocals": false,
    "noUnusedParameters": false,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": false,
    "inlineSourceMap": true,
    "inlineSources": true,
    "experimentalDecorators": true,
    "strictPropertyInitialization": false,
    "typeRoots": ["./node_modules/@types"]
  },
  "exclude": ["node_modules", "cdk.out"]
}

Additional Information/Context

package.json de. Dependencies should remain as is in current behavior:

"devDependencies": {
    "@types/jest": "^29.5.11",
    "@types/node": "20.10.4",
    "jest": "^29.7.0",
    "ts-jest": "^29.1.1",
    "aws-cdk": "2.115.0",
    "ts-node": "^10.9.2",
    "typescript": "~5.3.3"
  },

CDK CLI Version

2.115.0 (build 58027ee)

Framework Version

No response

Node.js Version

v20.10.0

OS

Linux Ubuntu

Language

TypeScript

Language Version

typescript: 5.3.3

Other information

No response

@niko-achilles niko-achilles added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 19, 2023
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Dec 19, 2023
@niko-achilles niko-achilles changed the title cdk init : tsconfig has DOM as bundled librray cdk init : tsconfig has DOM as bundled library Dec 19, 2023
@khushail
Copy link
Contributor

khushail commented Dec 19, 2023

Thanks @niko-achilles for reporting this. I can see the "dom" lib being added.
Adding the required labels for review by the team

@khushail khushail added p2 needs-review effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Dec 19, 2023
@niko-achilles
Copy link
Author

@khushail in the meantime i have found the location of the cause, here

As i can see DOM is added not only when a developer wants to create a cdk app but also when a developer wants to create a cdk lib. See here.

I can contribute by correcting the code lines at the link locations mentioned.
Some guidance/tips from cdk team side is needed on how to test the new behavior without "DOM" as lib declaration .

@khushail
Copy link
Contributor

khushail commented Dec 20, 2023

@niko-achilles , This PR introduced this change a long time ago and the reason for adding "DOM" is mentioned here.

@niko-achilles
Copy link
Author

@khushail thanks for the detailed information.
I invested some hours on the topic about workerpool and support of Web Workers API in Node.js and Support of transferable objects (only for web workers and worker_threads)
worker_threads are available in nodejs.
History of woker_threads support in nodejs.

In my investigation important information is that the author of the workerpool module that cdk depends has fixed TS warnings
and a new version of workerpool module, v9.0.4 is released.

See Links as reference to derive knowledge and assert the behavior:

Link 2 has a note which i duplicate in this comment:

Important note on 'workerType': when sending and receiving primitive data types (plain JSON) from and to a worker, the different worker types ('web', 'process', 'thread') can be used interchangeably. However, when using more advanced data types like buffers, the API and returned results can vary. In these cases, it is best not to use the 'auto' setting but have a fixed 'workerType' and good unit testing in place.

If the workerpool module is updated to v9.0.4 of the cdk repo, then the tsconfig definition in the repo of cdk may look as follows:

{
  "compilerOptions": {
    "target": "ES2020",
    "module": "commonjs",
    "lib": ["es2020"],
   ...
  },
  ...
}

I am pinging also the contributor of the PR mentioned in your comment , @rix0rrr for better visibility and decision.

Let me know if a new PR can be introduced.

@pahud pahud added the @aws-cdk/core Related to core CDK functionality label Jun 3, 2024
@comcalvi comcalvi added p3 and removed p2 labels Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort p3 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

No branches or pull requests

4 participants