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(schema): add vueCompilerOptions property to TSConfig in defineNuxtConfig #25924

Merged

Conversation

Evobaso-J
Copy link
Contributor

πŸ”— Linked issue

#25874

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Resolves #25874

VueCompilerOptions type added to the defineNuxtConfig schema

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have added tests (if possible).
  • I have updated the documentation accordingly.

Copy link

stackblitz bot commented Feb 22, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@Evobaso-J Evobaso-J changed the title fix(typescript): add vueCompilerOptions property to TSConfig in defineNuxtConfig fix: add vueCompilerOptions property to TSConfig in defineNuxtConfig Feb 22, 2024
@Evobaso-J Evobaso-J force-pushed the fix-add-vuecompileroptions-to-definenuxtconfig branch 2 times, most recently from 726ab6e to 88db8f2 Compare February 22, 2024 21:41
@Evobaso-J Evobaso-J changed the title fix: add vueCompilerOptions property to TSConfig in defineNuxtConfig fix(schema): add vueCompilerOptions property to TSConfig in defineNuxtConfig Feb 22, 2024
@Evobaso-J
Copy link
Contributor Author

@manniL Hi there!
This one is ready for code review. However, I'm not sure whether documentation on the VueCompilerOptions is needed or if a type addition, in general, is self-explanatory πŸ€”

By the way, I must say I couldn't find any reference to these options anywhere but on Volar's changelog, so having those documented somewhere could be nice!

Please let me know what you think ✌️
TY

packages/schema/build.config.ts Outdated Show resolved Hide resolved
Comment on lines 23 to 25
export type VueTSConfig = TSConfig & {
vueCompilerOptions?: VueCompilerOptions;
};
Copy link
Member

Choose a reason for hiding this comment

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

If the user doesn't have this dependency installed, this (and mention above in schema) will be typed as any. We could use something like this type:

type IsAny<T> = 0 extends 1 & T ? true : false

to conditionally augment tsConfig only when VueCompilerOptions is not any...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danielroe changes made!
I've been pondering about it for some days now but I still don't quite understand one thing: how can the user not have the dependency installed if they're installing nuxt? πŸ€”
Also, why don't we need to make the same check for other dependencies as well?

Copy link
Member

Choose a reason for hiding this comment

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

The user will only have this dependency installed if they add vue-tsc. Nuxt doesn't ship with vue-tsc by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I see! Thank you πŸ˜„

@Evobaso-J Evobaso-J force-pushed the fix-add-vuecompileroptions-to-definenuxtconfig branch from 8f14cfb to e6d2d72 Compare February 23, 2024 13:04
@Evobaso-J Evobaso-J marked this pull request as draft February 23, 2024 13:05
@Evobaso-J Evobaso-J force-pushed the fix-add-vuecompileroptions-to-definenuxtconfig branch from e6d2d72 to 0cc2014 Compare February 23, 2024 13:06
@Evobaso-J Evobaso-J force-pushed the fix-add-vuecompileroptions-to-definenuxtconfig branch from 4c9e58b to c05fb60 Compare February 26, 2024 19:52
@Evobaso-J Evobaso-J marked this pull request as ready for review February 26, 2024 19:55
@danielroe danielroe merged commit 3258cac into nuxt:main Feb 27, 2024
34 checks passed
@github-actions github-actions bot mentioned this pull request Feb 27, 2024
@Evobaso-J Evobaso-J deleted the fix-add-vuecompileroptions-to-definenuxtconfig branch February 27, 2024 20:10
@danielroe
Copy link
Member

Thank you ❀️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'vueCompilerOptions' does not exist in type 'tsConfig' in 'defineNuxtConfig'
2 participants