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

infra: reduce tsconfig complexity #2102

Merged
merged 1 commit into from
Jul 15, 2023
Merged

infra: reduce tsconfig complexity #2102

merged 1 commit into from
Jul 15, 2023

Conversation

Shinigami92
Copy link
Member

@Shinigami92 Shinigami92 commented Apr 27, 2023

This PR turns around the tsconfig files. Previously we had one tsconfig.base file, from which the others extends (but not the normal tsconfig.json).

Now we have one tsconfig.json which serves as the root/base config for all others!

This tsconfig.json now only configures the minimum amount of base for all the other configs.

Then we have three specific configs:

build-types defines what is required to generate the files from src to dist/types
we need to have this because it declares the "include: [src]" which is needed to tell tsc it only takes files from this folder.
I looked already if there is a CLI flag for --include, but sadly no.

then we have two (right now mostly identically) configs for scripts and tests.
they differ in their "include"
We could discuss if we want to merge them into one file and just use one CI step to check tests and scripts folder, or we could just move the configs into there specific folders
The later option would result in VSCode would read them and process the configs for the folders

eslint now takes the tsconfig.json instead of its own lint.config
this results in that we now see even more lint errors that were previously hidden (see https://github.com/faker-js/faker/actions/runs/4822808872/jobs/8590495983?pr=2102)


// tsconfig.json
{
  "compilerOptions": {
    "target": "ES2019", // the min target for Node v14.17, could potentially also moved into the tsconfig.build-types.json but it could affect eslint
    "moduleResolution": "node", // in TS v5+ we might want to set this to "bundler"
    "module": "ESNext", // the newest stuff
    "noEmit": true, // we dont want to emit any files by default, this will be overwritten in tsconfig.build-types.json
    "strict": true // we want strict by default
  },
  "exclude": ["node_modules", "dist", "locale"] // exclude node_modules and generated content by default
}
// tsconfig.build-types.json
{
  "extends": "./tsconfig.json", // extend from base
  "compilerOptions": {
    "noEmit": false, // we want to emit .d.ts files
    "declaration": true, // we want to emit .d.ts files
    "emitDeclarationOnly": true, // we want to emit .d.ts files only
    "stripInternal": true, // we want to strip @internal
    "rootDir": "src",
    "outDir": "dist/types",
    // We need to disable these for now, and need to tackle them in another PR
    "strictNullChecks": false,
    "strictBindCallApply": false
  },
  "include": ["src"] // this is important and the reason why we need this file!
}
// tsconfig.check-scripts.json and tsconfig.check-tests.json
{
  "extends": "./tsconfig.json",
  "compilerOptions": {
    "target": "ESNext", // we can use newer code in scripts and tests because it will not be shipped
    "allowSyntheticDefaultImports": true, // we import e.g. prettier.cjs
    "resolveJsonModule": true, // we e.g. import json files
    "skipLibCheck": true, // we access some vitepress and so on which throws invalid types but we dont care
    // We need to disable these for now, and need to tackle them in another PR
    "strictNullChecks": false,
    "strictBindCallApply": false,
    "noImplicitAny": false
  },
  "include": ["scripts"] || "include": ["tests"]
}	}

Edit:

we now have one tsconfig.json which serves for all files and then one specific for the build process

@Shinigami92 Shinigami92 added p: 1-normal Nothing urgent c: infra Changes to our infrastructure or project setup labels Apr 27, 2023
@Shinigami92 Shinigami92 self-assigned this Apr 27, 2023
@Shinigami92 Shinigami92 added the s: on hold Blocked by something or frozen to avoid conflicts label Apr 27, 2023
@Shinigami92
Copy link
Member Author

Waiting on #2099 to be merged

@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Apr 27, 2023

I looked already if there is a CLI flag for --include, but sadly no.

It's literally the default argument.
tsc src/** is the same as only tsc with include: ["src/**"] in the tsconfig.json.

@Shinigami92
Copy link
Member Author

I looked already if there is a CLI flag for --include, but sadly no.

It's literally the default argument. tsc src/** is the same as only tsc with include: ["src/**"] in the tsconfig.json.

This explains why I did not found it 🤦

@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Merging #2102 (12fa514) into next (c6323f8) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 12fa514 differs from pull request most recent head 5b3d955. Consider uploading reports for the commit 5b3d955 to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##             next    #2102    +/-   ##
========================================
  Coverage   99.59%   99.60%            
========================================
  Files        2643     2607    -36     
  Lines      245862   244977   -885     
  Branches     1151     1153     +2     
========================================
- Hits       244877   244001   -876     
+ Misses        958      949     -9     
  Partials       27       27            

see 68 files with indirect coverage changes

.github/workflows/ci.yml Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
src/modules/helpers/index.ts Outdated Show resolved Hide resolved
test/helpers.spec.ts Outdated Show resolved Hide resolved
test/scripts/apidoc/signature.example.ts Outdated Show resolved Hide resolved
test/support/seededRuns.ts Outdated Show resolved Hide resolved
vite.config.ts Outdated Show resolved Hide resolved
@Shinigami92 Shinigami92 force-pushed the improve-tsconfigs branch 3 times, most recently from 03de7b2 to daae0d6 Compare May 3, 2023 07:53
@Shinigami92 Shinigami92 changed the title infra: improve tsconfigs infra: only use two tsconfig files May 3, 2023
@Shinigami92 Shinigami92 marked this pull request as ready for review May 3, 2023 07:54
@Shinigami92 Shinigami92 requested a review from a team May 3, 2023 07:54
@Shinigami92 Shinigami92 requested a review from a team as a code owner May 3, 2023 07:54
@xDivisionByZerox
Copy link
Member

For the title: How about infra: reduce tsconfig complexity?

@Shinigami92 Shinigami92 changed the title infra: only use two tsconfig files infra: reduce tsconfig complexity May 3, 2023
tsconfig.json Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@Shinigami92 Shinigami92 marked this pull request as draft May 4, 2023 06:14
@Shinigami92 Shinigami92 force-pushed the improve-tsconfigs branch 2 times, most recently from 467ccc3 to 69d14a6 Compare May 4, 2023 08:11
@Shinigami92 Shinigami92 force-pushed the improve-tsconfigs branch from 69d14a6 to e2abe30 Compare May 4, 2023 08:11
@Shinigami92 Shinigami92 marked this pull request as ready for review May 4, 2023 08:24
package.json Show resolved Hide resolved
@Shinigami92 Shinigami92 removed the s: on hold Blocked by something or frozen to avoid conflicts label May 4, 2023
@Shinigami92
Copy link
Member Author

@ST-DDT ST-DDT added the s: needs decision Needs team/maintainer decision label May 6, 2023
@Shinigami92 Shinigami92 marked this pull request as draft July 14, 2023 20:38
@Shinigami92 Shinigami92 removed the s: needs decision Needs team/maintainer decision label Jul 14, 2023
@Shinigami92 Shinigami92 marked this pull request as ready for review July 14, 2023 21:55
@Shinigami92 Shinigami92 merged commit 0708eb4 into next Jul 15, 2023
@Shinigami92 Shinigami92 deleted the improve-tsconfigs branch July 15, 2023 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: infra Changes to our infrastructure or project setup p: 1-normal Nothing urgent
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants