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

fix: use --build for type-checking to be exhaustive and less-fragile #274

Merged
merged 8 commits into from
Dec 4, 2023

Conversation

haoqunjiang
Copy link
Member

@haoqunjiang haoqunjiang commented May 12, 2023

Fixes #267
Closes #181

Thanks @segevfiner for noticing this new feature!

Currently there's still a small annoyance that vue-tsc would output an extra error message when there're type errors in .vue files vuejs/language-tools#2622 But it works well if there's no error.

I've already submitted a PR to address it vuejs/language-tools#3176

Fixes #267

Thanks @segevfiner for noticing this new feature!

Currently there's still a small annoyance that `vue-tsc` would output
an extra error message when there're type errors in `.vue` files
vuejs/language-tools#2622 But it works well
if there's no error.

I've already submitted a PR to address it vuejs/language-tools#3176
@segevfiner
Copy link

Actually, they might have changed things so that it does run but doesn't type check packages with noEmit at a later releas...

Be sure we actually get errors with this. I have requested microsoft/TypeScript#53979 for them to make this work.

@haoqunjiang
Copy link
Member Author

What do you mean by "might have changed things"?

I tested this project https://github.com/sodatea/no-emit-test with both 5.0.4 and 5.1.0-dev.20230512, both reporting the same correct number of errors - though in subsequent calls the error location is lost for ./src/App.vue.
That can be fixed by removing *.tsbuildinfo each time before type-checking?

haoqunjiang added a commit that referenced this pull request May 12, 2023
Fixes #267

It's because I moved the `compilerOptions` from `tsconfig.json` to
`tsconfig.app.json` in the 2023-05-05 (v3.6.2) release, but forgot to
change the `type-check` script.

This isn't comprehensive because it doesn't check all the sub-projects,
but at least it's the same scope as previous versions.

Ideally I hope #274 can work reliably. But now I'm not sure. So let's
first fix this bug.
@haoqunjiang
Copy link
Member Author

That can be fixed by removing *.tsbuildinfo each time before type-checking?

vue-tsc --build --force seems to work quite reliably.

@segevfiner
Copy link

I tried configuring this myself once before but got no errors, but maybe I did something wrong or it was a different issue.

@segevfiner
Copy link

It might have been the --force thing

@haoqunjiang
Copy link
Member Author

haoqunjiang commented Nov 27, 2023

@cexbrayat This change could help catch the issue in #386
But I'm unsure if it's worth bringing the extra complexity to average users. The extraneous .tsbuildinfo files could be a surprise. And --force is a must, which is hard to explain…

Copy link
Member

@cexbrayat cexbrayat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave it a try, and I think it works well 👍

One note though: all the errors are logged twice. But I'm not sure if this is really an issue.

Copy link
Member

@cexbrayat cexbrayat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like typescript-jsx-nightwatch is failing when running pnpm test locally. There is probably a noEmit to add in another file

Copy link
Member

@cexbrayat cexbrayat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@haoqunjiang haoqunjiang merged commit 9b27fdf into main Dec 4, 2023
202 checks passed
@haoqunjiang haoqunjiang changed the title fix: use --build for type-checking to be exaustive and less-fragile fix: use --build for type-checking to be exhaustive and less-fragile Dec 4, 2023
@haoqunjiang haoqunjiang deleted the fix-type-check branch December 4, 2023 05:32
@bmulholland
Copy link

bmulholland commented Dec 5, 2023

FYI: I think this setting is incompatible with any JS in the project.

I have some legacy JS files in my app, so I have allowJS: true. If I try running type checking, I get "js emit is not supported." And if I add --noEmit again, "Compiler option '--noEmit' may not be used with '--build'"

kirillgashkov added a commit to kirillgashkov/yama that referenced this pull request Apr 4, 2024
- 'vue-tsc' uses '--build' because we are using 'references' in
  tsconfig.json.
- 'vue-tsc' doesn't use '--noEmit' because it is incompatible with
  '--build'. Instead we set 'noEmit' to 'true' in the referenced
  tsconfig.json files.
- 'vue-tsc' uses '--force' to make check-only builds more reliable (see
  vuejs/create-vue#274).
kirillgashkov added a commit to kirillgashkov/yama that referenced this pull request Jun 15, 2024
* Add package.json

* Add 'node_modules' to .gitignore

* Add 'typescript' to dev dependencies

* Configure TypeScript for Vue

* Configure TypeScript for Vite

* Set 'composite' in tsconfig.*.json

According to the documentation, the 'composite' option should be set to
'true' for referenced projects.

* Add Vite's client-side types to tsconfig.dom.json

* Add 'vite' to dev dependencies

* Add favicon.ico mock

* Add index.html

* Add vite.config.ts

* Fix tsconfig.dom.json

* Add Vite scripts

* Use a relative import for main.ts in index.html

- The main.ts reference is fundamentally different from the favicon.ico
  reference, Vite resolves references like this similarly to how JavaScript
  imports are resolved.
- '@/main.ts' in index.html that uses 'resolve.alias' works properly for the
  production build but doesn't work for the development server.

* Add 'vue-tsc' to dev dependencies

* Add 'vue' to dependencies

* Add '@vitejs/plugin-vue' to dev dependencies

* Add App.vue, main.ts

* Add '@types/node' to dev dependencies

Fixes TypeScript's "Cannot find type definition file for 'node'".

* Fix 'compilerOptions.paths' in tsconfig.dom.json

Fixes TypeScript's "Non-relative paths are not allowed when 'baseUrl' is not
set. Did you forget a leading './'?"

* Set 'noEmit' in referenced tsconfig.json files

This makes it possible to perform check-only builds of a project that
uses 'references' in tsconfig.json. The '--noEmit' flag cannot be used
here because it is incompatible with '--build'. Check-only builds are
especially important for 'vue-tsc' as it doesn't support emitting files.

* Add '*.tsbuildinfo' to .gitignore

* Add 'lint' script to package.json

- 'vue-tsc' uses '--build' because we are using 'references' in
  tsconfig.json.
- 'vue-tsc' doesn't use '--noEmit' because it is incompatible with
  '--build'. Instead we set 'noEmit' to 'true' in the referenced
  tsconfig.json files.
- 'vue-tsc' uses '--force' to make check-only builds more reliable (see
  vuejs/create-vue#274).

* Add 'eslint', 'eslint-plugin-vue', 'typescript-eslint' to dev dependencies

* Allow JavaScript to be checked in tsconfig.node.json

* Add eslint.config.js

* Add ESLint configuration recommended by 'typescript-eslint'

* Add '@types/eslint__js' to dev dependencies

* Stop using 'types' in tsconfig.json files

Because unfortunately, they don't work as expected.

* Enhance ESLint with type information

- '...tseslint.configs.recommendedTypeChecked' enables enhanced linting
  with type information.
- 'languageOptions.parserOptions.EXPERIMENTAL_useProjectService: true'
  allows 'typescript-eslint' to compute type information for a
  TypeScript project that uses 'references' in the tsconfig.json file
  (see typescript-eslint/typescript-eslint#2094).

* Add 'vue-eslint-parser' to dev dependencies

* Clean up eslint.config.js

* Include local type definitions in the project

* Declare 'eslint-plugin-vue' module

* Clean up eslint.config.js

* Add '@eslint/eslintrc' to dev dependencies

* Add '@types/eslint__eslintrc' to dev dependencies

* Configure ESLint for Vue

- 'eslint-plugin-vue' doesn't support flat configuration yet, therefore
  we rely on '@eslint/eslintrc' to use Vue's recommended configuration.

* Remove 'eslint-plugin-vue' module declaration

It is no longer directly imported in the project, so types are not
needed.

* Ignore a ESLint error regarding 'App.vue' being 'any'

* Add 'eslint-config-prettier' to dev dependencies

* Add 'prettier' to dev dependencies

* Add '@types/eslint-config-prettier' to dev dependencies

* Remove 'lint' from scripts

* Add 'check', 'format' to scripts

* Add Prettier's ESLint config

* Remove custom types from tsconfig.*.json

* Clean up tsconfig.node.json

* Remove 'types' field from tsconfig.json files

* Factor out tsconfig.base.json

* Add env.dom.d.ts

* Add 'tailwindcss', 'postcss', 'autoprefixer' to dev dependencies

* Initialize PostCSS and Tailwind

* Configure HTML template files for Tailwind

* Format

* Add types to postcss.config.js

* Add the Tailwind directives to style.css

* Add style.css import to main.ts

* Add Tailwind CSS Prettier plugin

* Add 'dist' to .gitignore

* Enable reporting of unused disable directives in ESLint

* Add vue-router to dependencies

* Add router

* Add pinia to dependencies

* Add Pinia

* Ignore dist in eslint

* Add @eslint/js to dev dependencies

* Suppress the createApp eslint error

* Remove the counter store
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Npm script type-check is not working
4 participants