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: RuntimeModuleHooks should be ModuleRuntimeHooks #194

Merged
merged 4 commits into from Dec 19, 2023

Conversation

BobbieGoede
Copy link
Member

@BobbieGoede BobbieGoede commented Nov 16, 2023

Can't believe I wrote RuntimeModuleHooks instead of ModuleRuntimeHooks in #183 😭 I added a deprecation warning, and kept the previous way working, let me know if this should be different or removed!

The previous code was not broken, but I feel like the code looks off

interface ModuleOptions {}
interface ModuleHooks {}
interface RuntimeModuleHooks {}
interface ModuleRuntimeConfig {}
interface ModulePublicRuntimeConfig {}

vs

interface ModuleOptions {}
interface ModuleHooks {}
interface ModuleRuntimeHooks {}
interface ModuleRuntimeConfig {}
interface ModulePublicRuntimeConfig {}

@userquin
Copy link
Member

That's why I get weird types.d.ts, check last pr from Daniel and the test snapshots.

moduleImports.push('RuntimeModuleHooks')
appShims.push(' interface RuntimeNuxtHooks extends RuntimeModuleHooks {}')
}
if (hasTypeExport('ModuleRuntimeHooks')) {
Copy link
Member

@userquin userquin Nov 17, 2023

Choose a reason for hiding this comment

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

Shouldn't throw an error when RuntimeModuleHooks also present?

Copy link
Member Author

Choose a reason for hiding this comment

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

After testing locally the types seem to work fine with two extends as seen below 🤷‍♂️ it feels like it shouldn't.. Might test it again in the morning just to be sure 😅

declare module '#app' {
  interface RuntimeNuxtHooks extends RuntimeModuleHooks {}
  interface RuntimeNuxtHooks extends ModuleRuntimeHooks {}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I've changed it so that if multiple interfaces are used it's extended like so:

declare module '#app' {
  interface RuntimeNuxtHooks extends RuntimeModuleHooks, ModuleRuntimeHooks {}
}

src/commands/build.ts Outdated Show resolved Hide resolved
@BobbieGoede BobbieGoede force-pushed the fix/module-runtime-hook-spelling branch 2 times, most recently from ba6a530 to 1c18f45 Compare November 23, 2023 08:47
@BobbieGoede BobbieGoede force-pushed the fix/module-runtime-hook-spelling branch from 1c18f45 to ebcf9e4 Compare November 23, 2023 08:49
@danielroe danielroe merged commit 6641d8d into nuxt:main Dec 19, 2023
1 check passed
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.

None yet

4 participants