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

[Vite] add a target for type checking #13954

Closed
1 task
vicb opened this issue Dec 21, 2022 · 14 comments
Closed
1 task

[Vite] add a target for type checking #13954

vicb opened this issue Dec 21, 2022 · 14 comments

Comments

@vicb
Copy link
Contributor

vicb commented Dec 21, 2022

Description

It would be great to have a typecheck target when vite is used to check the types.

Probably build:production should depend on this target.

Motivation

If you create a vite app and set the content of main.ts to:

function error(param: number) {
    console.log(param);
}

error("abc");

export {};

(Notice "abc" vs the expected number type.)

The build target will happily compiles this code.

While it's great to skip type check in to have a better (=faster) dev experience there should be a way to check the types and it would be nice to do it when building the production configuration.

Suggested Implementation

Use tsc --noEmit to check types.

@AgentEnder AgentEnder added the scope: bundlers Issues related to webpack, rollup label Dec 22, 2022
@FrozenPandaz FrozenPandaz added scope: js and removed scope: bundlers Issues related to webpack, rollup labels Dec 22, 2022
@mandarini
Copy link
Member

Hi there @vicb ! Thanks for creating this feature request. I see you are happy to implement this feature. Do you want to give this a shot, and assign the PR to me for review? I can help you if you need!

@vicb
Copy link
Contributor Author

vicb commented Jan 3, 2023

I could try to take a look at that but I will certainly need help.

As I'm getting more familiar with Nx I think the steps would be:

  1. Add a typecheck executor in the JS package,
  2. Generate a typecheck target using this executor from in the vite:configuration generator.

Would that be a good start?

@mandarini
Copy link
Member

How would you do this typechecking outside of Nx context, if you had a pure React+Vitejs app? I mean, if it's a script you set up, maybe there's no need to create a new executor. Maybe you can just create a new target that calls run-commands. But I am not sure how this would be set up.

@vicb
Copy link
Contributor Author

vicb commented Jan 3, 2023

How would you do this typechecking outside of Nx context, if you had a pure React+Vitejs app?

Using tsc --noEmit

Maybe there's no need to create a new executor

Creating an executor could be helpful whenever babel or swc are used to compile the code as those wouldn't do any type checking.

An other option that I just found out would be to use vite-plugin-checker. It is less generic as it would only apply to vite projects but it might be simpler?

WDYT?

@mandarini
Copy link
Member

Yes, I think the vite-plugin-checker seems like a better option in this case.

@vicb
Copy link
Contributor Author

vicb commented Jan 3, 2023

vite-plugin-checker after updating the plugins config as:

// vite.config.ts
import { checker } from 'vite-plugin-checker';

// ...

  plugins: [
    checker({
      typescript: {
        root: `${process.cwd()}/path/to/app`,
        tsconfigPath: 'tsconfig.app.json',
      },
    }),
    viteTsConfigPaths({
      root: '../../',
    }),
  ],

requires

npm i -D vite-plugin-checker

edit: added root for the build target to work as well

@mandarini
Copy link
Member

mandarini commented Jan 4, 2023

A typecheck target could look like:

    "vite-typecheck": {
      "executor": "nx:run-commands",
      "options": {
        "command": "tsc --noEmit -p <project-root>/tsconfig.app.json"
      }
    }

and maybe in that case you would not need that plugin? Or maybe I'm missing something.

You can check out this PR which does something along these lines.

@vicb
Copy link
Contributor Author

vicb commented Jan 4, 2023

Hey @mandarini do you want me to work on a PR along the lines of your last message?
My understanding from your previous comment was that we should use the vite plugin instead?

@mandarini
Copy link
Member

mandarini commented Jan 4, 2023

Hello @vicb ! Yes, sure, work on that PR! :) Thank you very much!!!

@vicb
Copy link
Contributor Author

vicb commented Jan 18, 2023

We can probably use https://github.com/nrwl/nx/blob/master/packages/js/src/utils/typescript/run-type-check.ts

I'll take a look at that next

@jonybekov
Copy link

@Lonli-Lokli
Copy link

Lonli-Lokli commented Nov 22, 2023

According to the documentation https://nx.dev/nx-api/vite/executors/build#skiptypecheck it should just work. It has been added https://github.com/nrwl/nx/releases/tag/16.4.0-beta.3

@mandarini
Copy link
Member

I am closing this issue because indeed it should work, and we are shifting things around with targets, too, at the moment.

Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants