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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 8 commits into from Dec 4, 2023
Merged

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

merged 8 commits into from Dec 4, 2023

Conversation

sodatea
Copy link
Member

@sodatea sodatea commented May 12, 2023 •
edited

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

@sodatea
fix: use --build for type-checking to be exaustive and less-fragile
b13089f 
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
@sodatea sodatea mentioned this pull request May 12, 2023
Closed
@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.

@sodatea
Copy link
Member Author

sodatea commented May 12, 2023

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?

sodatea added a commit that referenced this pull request May 12, 2023
@sodatea
fix: specify the sub-project to type-check
eb8ad5d 
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.
@sodatea sodatea mentioned this pull request May 12, 2023
Merged
@sodatea
Copy link
Member Author

sodatea commented May 12, 2023

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

@sodatea sodatea mentioned this pull request Jul 25, 2023
Closed
@sodatea
Copy link
Member Author

sodatea commented Nov 27, 2023 •
edited

@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…

cexbrayat
cexbrayat approved these changes Nov 29, 2023
View reviewed changes
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.

cexbrayat
cexbrayat requested changes Nov 29, 2023
View reviewed changes
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

cexbrayat
cexbrayat approved these changes Nov 30, 2023
View reviewed changes
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.

👍

@sodatea sodatea merged commit 9b27fdf into main Dec 4, 2023
202 checks passed
@sodatea sodatea 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
@sodatea sodatea deleted the fix-type-check branch December 4, 2023 05:32
@sodatea sodatea mentioned this pull request Dec 4, 2023
Open
@bmulholland
Copy link

bmulholland commented Dec 5, 2023 •
edited

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'"

@unshame unshame mentioned this pull request Jan 31, 2024
Open
@sapphi-red sapphi-red mentioned this pull request Feb 20, 2024
Open
4 tasks
@nighca nighca mentioned this pull request Mar 1, 2024
Merged
kirillgashkov added a commit to kirillgashkov/yama that referenced this pull request Apr 4, 2024
@kirillgashkov
Add 'lint' script to package.json
adf88dc 
- '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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cexbrayat cexbrayat cexbrayat approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Npm script type-check is not working
4 participants
@sodatea @segevfiner @bmulholland @cexbrayat