-
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
Missing typescript checks when using inferred tasks (project Crystal) and vite #27501
Comments
@vtregner-cen29414 As you've mentioned yourself, TS Checks are not part of the Project Crystal's intention is to use the underlying tooling more correctly. I'll add a typecheck target that gets inferred by the vite plugin, but it'll be a separate target.
Then in the
Even if we infer the |
<!-- Please make sure you have read the submission guidelines before posting an PR --> <!-- https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr --> <!-- Please make sure that your commit message follows our format --> <!-- Example: `fix(nx): must begin with lowercase` --> <!-- If this is a particularly complex change or feature addition, you can request a dedicated Nx release for this pull request branch. Mention someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they will confirm if the PR warrants its own release for testing purposes, and generate it for you if appropriate. --> ## Current Behavior <!-- This is the behavior we have today --> Vite intentionally does not run typechecking on projects. They recommend running `tsc --noEmit` separately. The `@nx/vite/plugin` could make this easier by adding a `typecheck` when it detects a `tsconfig` file in the `projectRoot`. This can then be added to the build pipeline on CI. Or users can add it to a `dependsOn` for the `build` target if they wish. ## Expected Behavior <!-- This is the behavior we should expect with the changes in this PR --> The `@nx/vite/plugin` could make this easier by adding a `typecheck` when it detects a `tsconfig` file in the `projectRoot`. This can then be added to the build pipeline on CI. Or users can add it to a `dependsOn` for the `build` target if they wish. ## Related Issue(s) <!-- Please link the issue being fixed so it gets closed when this is merged. --> Fixes #27501 (cherry picked from commit 6d963fd)
@Coly010 The inferred typecheck PR doesn't load any tsconfig file. The command |
Good catch |
<!-- Please make sure you have read the submission guidelines before posting an PR --> <!-- https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr --> <!-- Please make sure that your commit message follows our format --> <!-- Example: `fix(nx): must begin with lowercase` --> <!-- If this is a particularly complex change or feature addition, you can request a dedicated Nx release for this pull request branch. Mention someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they will confirm if the PR warrants its own release for testing purposes, and generate it for you if appropriate. --> ## Current Behavior <!-- This is the behavior we have today --> The vite plugin's typecheck target is not using the tsconfig.lib.json project when running typecheck for libs ## Expected Behavior <!-- This is the behavior we should expect with the changes in this PR --> It should use the correct tsconfig ## Related Issue(s) <!-- Please link the issue being fixed so it gets closed when this is merged. --> Fixes #27501
<!-- Please make sure you have read the submission guidelines before posting an PR --> <!-- https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr --> <!-- Please make sure that your commit message follows our format --> <!-- Example: `fix(nx): must begin with lowercase` --> <!-- If this is a particularly complex change or feature addition, you can request a dedicated Nx release for this pull request branch. Mention someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they will confirm if the PR warrants its own release for testing purposes, and generate it for you if appropriate. --> ## Current Behavior <!-- This is the behavior we have today --> The vite plugin's typecheck target is not using the tsconfig.lib.json project when running typecheck for libs ## Expected Behavior <!-- This is the behavior we should expect with the changes in this PR --> It should use the correct tsconfig ## Related Issue(s) <!-- Please link the issue being fixed so it gets closed when this is merged. --> Fixes #27501 (cherry picked from commit 5648344)
This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context. |
Current Behavior
When new monorepo with react application and vite bundler is generated, build task doesn't include Typescript checks. Build is successful even if contains typescript errrors.
Expected Behavior
Build of application with Typescript errors should fail.
GitHub Repo
No response
Steps to Reproduce
Generate repo using npx create-nx-workspace
and with these parameters:
√ Where would you like to create your workspace? · org
√ Which stack do you want to use? · react
√ What framework would you like to use? · none
√ Integrated monorepo, or standalone project? · integrated
√ Application name · org
√ Which bundler would you like to use? · vite
√ Test runner to use for end to end (E2E) tests · none
√ Default stylesheet format · scss
√ Which CI provider would you like to use? · skip
√ Would you like remote caching to make your build faster? · skip
Type some code with typescript error, for example add this line to app.tsx:
const shouldShowError: string = 5;
Build application
nx run org:build
Nx Report
Failure Logs
No response
Package Manager Version
No response
Operating System
Additional Information
Issue where missing typescript checks was solved in past (prior projet Crystal)
#13954
The Vite discussion points out that TS checks are not part of the Vite build process:
vitejs/vite#12870
https://vitejs.dev/guide/features.html#transpile-only
The text was updated successfully, but these errors were encountered: