-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Type check command #3664
Comments
I guess a third option is writing a problem matcher for the build cli command? |
Build command can not cover the code that is not used in any app yet, so a typescript checker is necessary |
typescript-eslint/typescript-eslint#352 |
Currently I use |
+1 to see a proper solution to just running type checks. I would love to separate my build from type checks in order to parallize them as well as enabling the possibility of running different bundlers that don't do the type checking as well. @zhangciwu can you elaborate a little bit more on that setup? I tried to achieve something similar, by adding a run-command to all apps / projects e.g.
The problem is when running it for some libs, which is when I am getting the rootDir error: The final issue I am stuck here with, that this does not seem to pick up the proper extended tsconfig in my referenced projects. Since I am getting this Did you manually run through this whole process or is there a more holistic solution? |
@KeKs0r Tried with nx command and it works well, the command I use is:
Some points:
|
I agree we should have this command @KeKs0r Did you make it? If so, can you share your configuration? I tried to configure my app // myapp/tsconfig.json
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"types": ["node", "jest"]
},
"include": ["**/*.ts"],
"references": [
{
"path": "./tsconfig.app.json"
}
]
}
// myapp/tsconfig.app.json
{
"extends": "./tsconfig.json",
"compilerOptions": {
"outDir": "../../dist/out-tsc",
"emitDecoratorMetadata": true,
"module": "commonjs",
"target": "es2017",
"types": ["node"]
},
"exclude": ["**/*.spec.ts"],
"references": [
{
"path": "../../libs/lib1/tsconfig.json"
},
// <-- ... long list of libs refrence
]
} without referencing libs
@zhangciwu Is this how it should be? |
@AliYusuf95 I put all the lib path config in root tsconfig, and using "paths": {
"@myproject/testlib": ["libs/testlib/src/index.ts"],
} Also I have blank |
@AliYusuf95 : unfortunately not. There was a chain of TS issues I ran into and could not resolve them. |
I created this executor which worked for my use case and then you can run standard commands such as |
I wrote up a blog post on how we are using TypeScript project references with NX at https://jakeginnivan.medium.com/using-typescript-project-references-in-nx-b3462b2fe6d4 The tl;dr of it is:
|
@JakeGinnivan that looks nice but I am not sure if the solution is to not use nrwl's app plugins. 😅 Although I totally understand that this may take months to be resolved... I know this is open source and everything but do people really use in production something that doesn't tell them typescript errors by default? We can always write our own solution but that kind of kills the point of "standardization". 🙂 I focus on code quality and this is a major blocker for me. Every project should have:
otherwise it should never make it to production. 🙉 |
This looks like a good solution although looking at the documentation the executor needs more than 1 file to actually work. 🤔 |
@developer239 I agree with the standardization and minimum quality bar, which is why we created our own plugin set. The goal for us is to only type check once, not multiple times and also get a great editor and developer experience. The advantage of our approach is not only do we only type check once, we get full support for incremental compilation with TypeScript project references so the incremental compilation is really fast so we get super quick type checking feedback in our editors as we are developing. Another advantage of us creating a new set of plugins is that the nrwl team can check out the different approach rather than any discussions just being based on theory. |
@JakeGinnivan My post was more like a rhetorical question to the maintainers of this project. I totally understand why you decided to use your own solution. 🙌 |
@developer239 totally, I hope that they a look at my solution and consider it for a way forward |
@JakeGinnivan awesome work! i've been looking at your plugin and method as a starting point for a similar approach in our monorepo. one question, when you say:
could you elaborate on how you configured VSCode? i am not super familiar with running background tasks and how to make sure VSCode is using the
is working great and is really 🔥 |
For sure, the language service only really picks up issues of open files, but it also provides intellisense and such so the approach doesn't replace it, it just augments it. This is my
|
@JakeGinnivan import { ExecutorContext } from '@nrwl/devkit';
import { detectPackageManager } from '@nrwl/tao/src/shared/package-manager';
import { spawn } from 'child_process';
export default async function tscExecutor(_, context: ExecutorContext) {
const packageManager = detectPackageManager();
const packageManagerCmd = packageManager === 'pnpm' ? 'pnpx' : packageManager === 'yarn' ? 'yarn' : 'npx';
const libRoot = context.workspace.projects[context.projectName].root;
const executionCode = await new Promise((resolve) => {
const child = spawn(packageManagerCmd, ['tsc', '-p', libRoot, '--noEmit'], { stdio: 'inherit' });
child.on('data', (args) => console.log(args));
child.on('close', (code) => resolve(code));
});
return { success: executionCode === 0 };
} |
I'm trying to replicate your executor but when I run
Do you how to solve it? |
I did not test your code but I think |
Oh yeah, it was that, thank you! Do you know if there is something else that I need to setup (except the steps in the docs for building custom executors)? Currently the command doesn't throw any error but if I run |
change this line in your impl.ts and build ts file again |
@JakeGinnivan Again - thank you 👍 For anybody interested: this is a repo with working command
{
"targets": {
"tsCheck": {
"executor": "./tools/executors/tsCheck:tsCheck"
}
}
}
|
For Windows users a slight fix is needed. Add import { ExecutorContext } from '@nrwl/devkit';
import { detectPackageManager } from '@nrwl/tao/src/shared/package-manager';
import { spawn } from 'child_process';
export default async function tscExecutor(_, context: ExecutorContext) {
const packageManager = detectPackageManager();
const packageManagerCmd = packageManager === 'pnpm' ? 'pnpx' : packageManager === 'yarn' ? 'yarn' : 'npx';
const libRoot = context.workspace.projects[context.projectName].root;
const executionCode = await new Promise((resolve) => {
const child = spawn(packageManagerCmd, ['tsc', '-p', libRoot, '--noEmit'], {
stdio: 'inherit',
shell: true // Windows fix "Error: spawn npx ENOENT"
});
child.on('data', (args) => console.log(args));
child.on('close', (code) => resolve(code));
});
return { success: executionCode === 0 };
} |
On my side something is off, when running the upper executor on a project it will detect assets like .svg's as errors ' The upper script will use the tsconfig from a specific project even for the shared stuff across the repo, I am not yet sure how to fix this, but in the IDE (VSCode) I don't seem to have the same issues. |
Since I've found the solution here and implemented it in our project, I thought I might just as well push and publish it: https://github.com/webpro/nx-tsc. This will only type-check using tl/dr;
{
"$schema": "../../node_modules/nx/schemas/project-schema.json",
"sourceRoot": "libs/my-lib/src",
"targets": {
"tsc": {
"executor": "@webpro/nx-tsc:tsc",
"options": {
"tsConfig": ["tsconfig.json"]
}
}
}
}
Thanks, and maybe it's useful to someone. Feel free to file any issues/questions in the GH repo. |
@webpro this works but doesn't output the errors to the terminal when doing |
The current solution uses |
You can also do this without additional libraries with {
"$schema": "../../node_modules/nx/schemas/project-schema.json",
"sourceRoot": "libs/my-lib/src",
"targets": {
"tsc": {
"executor": "nx:run-commands",
"options": {
"commands": [
{
"command": "tsc --noEmit -p libs/my-lib/tsconfig.json"
}
]
}
}
}
} |
This solution works for me as well and I have generator that add this command to project.json. But I use |
Didn't measure the difference, but wanted to eliminate anything I could since spawning + running |
@webpro your plugin works great! I'm going to replace our custom commands with it. Would you say this is more performant than running the command directly, when running As far as the whole "it's another dependency" caveat... I don't place too much stock in that. The whole entire approach with executors in Nx has you downloading another dependency, for the purpose of standardization, simplification, etc. There's a lot of benefits of these things being in another dependency as well, because if you're in a big company, someone may be tempted to sneak some flag or custom config into one of the projects, because the executor is for custom commands. So it being a dependency is to an extent, a plus for me. It ensures this will stay simple & standard across packages. Granted, having official support would be nice, but this is the next best thing. Thanks for making it 🙌 |
@webpro using your plugin too. Works great, thank you ! 👏 Usage that works in my project : Followed instructions in
Then added as an npm script in
Added as a custom build task in VSCode's
The --verbose flag is not required for VSCode (VSCode catches the intermediate outputs of NX and give them to $tsc problem matcher) but required when used in the command-line, or else you won't be able to see the problematic files and line numbers. In my opinion, this kind of feature should be built-in in NX. Builds must fail if there are problems in typings and not be shipped in production (like we do in every other typed language). |
I don't think so. In both cases you're spawning one process per each project you type check, the only difference is the verbosity of the configuration. Note, adding this new Nx target may have some overlap with the type checking already being done in the I think vite, etc. seems like a potential distraction. Really the "ask" here is for Alternatively, In the meantime the workaround is pretty simple, just add somewhat redundant targets to your project.json:
The few seconds of redundant CPU cycles is not worth hours of your time fixing bugs months later. It would be nice if nx could update the |
I recently added targets to our projects to perform type checking. I wanted to create a reusable executor, instead of creating the redundant targets. This way if I need to make a change to the command, I can make sure it is the same for all. Here is what I have:
You are talking about a few seconds of redundant CPU cycles @joshribakoff-sm, but in my experience the run time for my type checking target is around 13 minutes for 168 small projects in my workspace when running in our Github Workflow. Locally it only takes 2 minutes. Caching the target helps in many cases, and |
Seems unrelated to this thread. Maybe try increasing the resources for your CI machines.
Personally, I would rather wait 13 minutes or however long, rather than letting bugs slip through, but that's just my thought process and I'm not suggesting everyone should do it the same way :) If skipping type checking is desirable, that's a valid choice to make but not the choice I like to make.
Interesting, I'm curious if it surfaces the logs, or just the exit code - looks pretty nice though! |
The performance of things is indeed not really what this issue is about, but part of the solution is to prevent doing things. In a recent project half of the Nx projects consisted of Next.js applications. So for such cases where you build applications and they're type-checked in such a process as well, try not to type-check the app/lib twice. |
This command works for me pretty well:
|
@leosvelperez is working on something in this area. Stay tuned. |
Hi everyone, Is there any update on this? |
Maybe create your own generator? |
I hoped that there was an easier way of doing this since typechecking should be one of the basic features in a build system tool like NX. It's also important to note that a generator can only partially fix the issue because you don't have to manually add the target when you create a library but you will still have a huge amount of duplicated config code for the type cheking generator in your entire workspace |
works nicely , can't we use a placeholder here |
I wanted to try and create a local plugin that would infer these typechecking tasks for any project with a tsconfig.json, but trying to understand how to do that when the only option is essentially reading the vite/jest plugins which already do this and trying to figure out what APIs they're using are even public at all has put it a bit beyond me at the moment. Has anyone else attempted this? |
What is the current status on this?
Has something been released already or should we roll our own for now? |
@Shuunen is that something you can open source? |
I did nothing ^^ it's comming from the nx/vite plugin 😸 |
I have noticed that the new Are you sure that the For example, in the custom executor I made I check the libType:
|
I see the following target inferred for my project: Which indicates I should be able to pass any flag from However, adding "typecheck": {
"options": {
"project": "tsconfig.json"
}
}, gives: > nx run web:typecheck
> tsc --noEmit -p tsconfig.app.json --project=tsconfig.json
error TS5023: Unknown compiler option '--project=tsconfig.json'.
Warning: command "tsc --noEmit -p tsconfig.app.json --project=tsconfig.json" exited with non-zero status code |
Any way to make typecheck also run checks for spec / test / storybook files that use separate tsconfig files? |
@ryan-mcginty-alation You can do something like this... {
// ...
"typecheck": {
"executor": "nx:noop",
"dependsOn": ["typecheck-app", "typecheck-spec", "typecheck-storybook"]
},
"typecheck-app": {
"executor": "nx:run-commands",
"options": {
"command": "tsc --noEmit -p {projectRoot}/tsconfig.app.json"
}
},
"typecheck-spec": {
"executor": "nx:run-commands",
"options": {
"command": "tsc --noEmit -p {projectRoot}/tsconfig.spec.json"
}
},
"typecheck-storybook": {
"executor": "nx:run-commands",
"options": {
"command": "tsc --noEmit -p {projectRoot}/tsconfig.storybook.json"
}
}
} |
Description
One way I leverage TypeScript is when making a behavioral change to my project is to ensure the source of the change is a compilation mistake.
Being able to run a type check over the entire project through VS Code and use the TypeScript problem matcher to ensure they are surfaced in the
Problems
tabMotivation
Being able to surface all build errors in a repo is very handy, especially when making a change to one to the shared projects.
Suggested Implementation
Until the solution file exists, running
tsc -p apps/*/tsconfig.json --noEmit --incremental
for each app tsconfig?Alternate Implementations
Maintain a solution tsconfig.all.json, I read there were some issues with having one so having it named differently should stop it being picked up by VSCode. I think we would then need
tsconfig.check.json
for each project which sets no-emit and incremental.The text was updated successfully, but these errors were encountered: