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: type correction for loading, loadingIndicator, transpile #1023

Merged
merged 2 commits into from
Dec 18, 2023

Conversation

wattanx
Copy link
Collaborator

@wattanx wattanx commented Dec 17, 2023

πŸ”— Linked issue

Fixes: #1019
Fixes: #1016

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 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)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

I will correct the types for loading, loadingIndicator, transpile in the nuxt.config.
I would like to add type tests in a separate PR.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@wattanx wattanx marked this pull request as ready for review December 17, 2023 14:33
* ```
* @type {Array<string | RegExp | ((ctx: { isClient?: boolean; isServer?: boolean; isDev: boolean }) => string | RegExp | false)>}
*/
transpile: {

Choose a reason for hiding this comment

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

Nice <3

@@ -254,6 +246,7 @@ export default defineUntypedSchema({
* configuration. The name can refer to an indicator from [SpinKit](https://tobiasahlin.com/spinkit/)
* or a path to an HTML template of the indicator source code (in this case, all the
* other options will be passed to the template).
* @type {typeof import('../src/types/loading').NuxtOptionsLoadingIndicator | false}
Copy link

Choose a reason for hiding this comment

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

It's still missing the type false isn't it ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have confirmed that setting false does not cause an error, what do you think is the problem?

Copy link

Choose a reason for hiding this comment

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

Here's the current issue I have
image

And here's the attribute in the node_modules/@nuxt/bridge-schema/dist/index.d.ts ConfigSchema interface

 /**
  * Used to set the default properties of the page transitions.
  * 
  * You can either pass a string (the transition name) or an object with properties to bind to the `<Transition>` component that will wrap your pages.
  * 
  * 
  * @see [vue@2 documentation](https://v2.vuejs.org/v2/guide/transitions.html)
  * 
  * @see [vue@3 documentation](https://vuejs.org/guide/built-ins/transition-group.html#enter-leave-transitions)
 */
 pageTransition: {
  [key: string]: any
 },

@danielroe danielroe merged commit ba2b18e into main Dec 18, 2023
9 checks passed
@danielroe danielroe deleted the fix/fix-config-type branch December 18, 2023 13:34
@github-actions github-actions bot mentioned this pull request Dec 18, 2023
@wJoenn
Copy link

wJoenn commented Dec 18, 2023

@danielroe I just discovered another type mismatch (currently installing bridge on a old Nuxt 2 app).
app.meta.script raises an error when a body: true attribute is passed to a script

app: {
  head: {
    script: [
      { src: `https://polyfill.io/v3/polyfill.min.js?features=${POLYFILL_FEATURES}`, body: true }
    ]
  }
}

image

But I can see in Unhead's documentation that

Providing body: true is the same as tagPosition: 'bodyClose'

tagPosition: 'bodyClose' does not raise an error. body: true does.

Shall I open a new issue for this one ?

@github-actions github-actions bot mentioned this pull request Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants