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(nuxt): merge options with public runtime config #204

Merged
merged 1 commit into from
Jun 22, 2024

Conversation

enkot
Copy link
Contributor

@enkot enkot commented Jun 21, 2024

πŸ”— Linked issue

❓ 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

Hi πŸ‘‹
Currently module options are not public, so config.public.motion is always undefined in the Vue plugin.
This PR makes options public.

πŸ“ Checklist

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

@enkot
Copy link
Contributor Author

enkot commented Jun 21, 2024

@Tahul @BobbieGoede Can we merge it soon? This is a fairly simple fix

@BobbieGoede
Copy link
Member

Good find, thanks!

I think this change fixes/allows users to also provide/exclude directives with the motion key in nuxt.config.ts, options provided through runtimeConfig.public.motion should have been working as expected https://github.com/vueuse/motion/blob/main/src/nuxt/runtime/templates/motion.ts#L8 without this change.

@BobbieGoede BobbieGoede changed the title fix(nuxt): make options public fix(nuxt): merge options with public runtime config Jun 22, 2024
@BobbieGoede BobbieGoede merged commit 54971c6 into vueuse:main Jun 22, 2024
5 checks passed
@enkot
Copy link
Contributor Author

enkot commented Jun 25, 2024

@BobbieGoede I think it makes sense to show the example with motion key and not runtimeConfig in the Installation section of the docs, WDYT?

// nuxt.config.ts
export default defineNuxtConfig({
  motion: {
    directives: {
      'pop-bottom': {
        initial: {
          scale: 0,
          opacity: 0,
          y: 100,
        },
        visible: {
          scale: 1,
          opacity: 1,
          y: 0,
        }
      }
    }
  }
})

Typically, runtimeConfig is an alternative, but not the primary option.

@BobbieGoede
Copy link
Member

I agree, runtimeConfig is nice but secondary to configuration by key and modules array, we should update the docs.

We should probably consider deprecating runtimeConfig for the next major version and instead support and recommend configuring directives/presets in app.config since changes will update reactively without restarting the Nuxt server.

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.

2 participants