Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

feat(nuxt3): add support for definePageMeta macro #2678

Merged
merged 28 commits into from
Jan 17, 2022
Merged

Conversation

danielroe
Copy link
Member

@danielroe danielroe commented Jan 11, 2022

πŸ”— Linked issue

https://github.com/nuxt/framework/discussions/2468

❓ 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

This PR is the first step in features enabled by 'extracting' metadata from components. I've included implementations of transition props and layouts via definePageMeta (though happy to extract the layouts change to another PR).

NOTE: at the moment HMR isn't working on route meta.

Transition props

It is now possible to define transition props which will be applied to the <transition> wrapping the page component (or pass false to disable it entirely):

<NuxtTransition :options="route.meta.transition ?? { name: 'page', mode: 'out-in' }">
<Suspense @pending="() => onSuspensePending(Component)" @resolve="() => onSuspenseResolved(Component)">
<component :is="Component" :key="route.path" />
</Suspense>
</NuxtTransition>

Layouts

This PR changes how layouts are defined. Previously they had to be defined via a component option on the top-level page (ie. not in a child route). Now, they can be defined in a parent or nested route, via definePageMeta({ layout }). This also now accepts a computed property or a ref (for reactive layout changes within a page).

<template>
  <div>
    <button @click="enableCustomLayout">Update layout</button>
  </div>
</template>

<script setup>
const route = useRoute()
function enableCustomLayout () {
  // Note: because it's within a ref, it will persist if 
  // you navigate away and then back to the page.
  route.layout.value = "custom"
}
definePageMeta({
  layout: ref(false),
});
</script>

Middleware

I will be following up shortly with a middleware implementation.

Route key

I will be following up shortly with a route key implementation.

πŸ“ Checklist

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

@danielroe danielroe self-assigned this Jan 11, 2022
@netlify
Copy link

netlify bot commented Jan 11, 2022

βœ”οΈ Deploy Preview for nuxt3-docs canceled.

πŸ”¨ Explore the source changes: 1160770

πŸ” Inspect the deploy log: https://app.netlify.com/sites/nuxt3-docs/deploys/61e5b4fd68f37100081b6f14

@danielroe danielroe requested a review from pi0 January 11, 2022 22:24
@tobiasdiez
Copy link
Contributor

I like it!

For the record, there is https://github.com/dword-design/nuxt-route-meta which provides similar features (also for nuxt2). Inspired by this library, maybe its worthwhile to also support the following syntax

export default defineNuxtComponent({
   pageMeta: {
      layout: 'custom',
      auth: true
   }
} 

to set the page metadata.

What I'm missing for the docs is how this is used with typescript, i.e. how to define a custom PageMeta object to make sure that one only uses 'known' metadata objects. For this the current implementation

export interface PageMeta {
  [key: string]: any
   ...
}

might lead to problems. For example, if I define CustomPageMeta extends PageMeta then it still accepts arbitrary keys.

@pi0
Copy link
Member

pi0 commented Jan 13, 2022

In general Looks good to me πŸ‘ But some notes:

  • I think using AST to extract micro object seems to be a much safer choice than regex extraction. (how does vue extracts for built-in micros does it use regex too?)
  • Considering we plan to extract Layout as a built-in component (Moving <NuxtLayout> to a built-in componentΒ nuxt#12829) does it still makes sense to define it in page meta micro?
    • Can we make micro implementation more generic (defineComponentMeta) and extend with a special definePageMeta case?

@pi0
Copy link
Member

pi0 commented Jan 13, 2022

@tobiasdiez Good point! I might be wrong but I guess when extending interface with types, typescript checks it over generic [key: string]: any:

interface PageMeta {
  [key: string]: any
}

interface CustomPageMeta extends PageMeta {
  custom: number
}

// Type 'string' is not assignable to type 'number'.ts(2322)
const meta: CustomPageMeta = { custom: 'string' }

But it might be also good idea if we use something like definePageMeta({ custom: {} }) for user-land config and more type safetely (also ensuring keys won't overlap if we introduce a new option that users already have custom use of!)

@tobiasdiez
Copy link
Contributor

tobiasdiez commented Jan 13, 2022

I agree that the more narrow types overwrite the generic keys. What I meant is that in your example

const meta: CustomPageMeta = { custom: 0, someotherkey: 'which I don't want' }

doesn't lead to a typescript error. Thus one cannot easily say "only the following keys are allowed" (or at least I don't know how to exclude the [key: string]: any part from the type).

@pi0
Copy link
Member

pi0 commented Jan 13, 2022

If we include a sub-namespace of custom: { }, rest of top-level keys supported by framework can be narrowed down to only known keys for typing without generic string: any.

@danielroe
Copy link
Member Author

A couple of comments - the macro plugin will allow us to define other macros, like defineComponentMeta or definePageMeta - or even something like defineLayout if we wished... The concept at the moment is that this metadata is set directly on $route.meta.

By doing this, we allow vue-router to handle the merging of metadata. But it also doesn't make sense to set route meta on components. So I'd suggest we can keep defineComponentMeta a separate macro for another use-case. It's easily enabled whenever we need it using this same plugin, though we'd likely want to extract it from the pages module at that point.

Another benefit is that we can declare types for these keys on RouteMeta which will be picked up by users using $route.meta throughout the app:

https://github.com/vuejs/vue-router-next/blob/c8430ea6c6033619041aadaa96527db6024ec66d/src/types/index.ts#L250-L266

Personally, I think the user DX of accessing something from e.g. route.meta.title is better than route.meta.custom.title and also avoids some edge-cases in terms of how vue-router merges the meta data. That is, the custom object on the child would override the entire custom object from the parent, meaning we don't get the benefit of this feature - so setting a titleTemplate on a parent route wouldn't make it into the child.

I'm happy to remove the layout implementation πŸ‘ However, I think we will still need something it even with <NuxtLayout> extracted from <NuxtPage> if we want layout values to be possibly defined per-route. Probably needs some engagement with the issue so we can make a decision on that before implementing...

packages/nuxt3/src/pages/macros.ts Outdated Show resolved Hide resolved
packages/nuxt3/src/pages/macros.ts Show resolved Hide resolved
packages/nuxt3/src/pages/runtime/composables.ts Outdated Show resolved Hide resolved
Co-authored-by: pooya parsa <pyapar@gmail.com>
@danielroe danielroe requested a review from pi0 January 17, 2022 15:47
@pi0 pi0 merged commit 93ef422 into main Jan 17, 2022
@pi0 pi0 deleted the feat/route-meta-macro branch January 17, 2022 18:27
export function normalizeRoutes (routes: NuxtPage[], metaImports: Set<string> = new Set()): { imports: Set<string>, routes: NuxtPage[]} {
return {
imports: metaImports,
routes: routes.map((route) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This can cause issues if people have components that don't export meta. Specifically, people will run into "The requested module ...' does not provide an export named 'meta'. That means every component will need to be updated to export const meta = {} at the least, which might not be great DX.

Copy link
Member Author

Choose a reason for hiding this comment

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

We provide a default meta export (equalling undefined) via the transform plugin.

Copy link
Contributor

@richardeschloss richardeschloss Jan 18, 2022

Choose a reason for hiding this comment

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

I think this PR should probably be revisited, as it introduced unintended effects, such as this one and failing to honor the layouts property in the component. That layouts prop now needs to be moved into the meta section. If that's the design, then the docs should be updated to alert people (users of the Options API) to the breaking changes.

Copy link
Member Author

@danielroe danielroe Jan 18, 2022

Choose a reason for hiding this comment

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

If you are experiencing a problem (you mention unintended side effects), would you be able to raise an issue?

Equally, I'd welcome any thoughts on the docs: http://v3.nuxtjs.org/docs/directory-structure/layouts

Copy link
Contributor

@richardeschloss richardeschloss Jan 18, 2022

Choose a reason for hiding this comment

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

Specifically, if I use the extendPages method, even if I specify meta to {}, I encounter this error:

""The requested module ...' does not provide an export named 'meta'"

My only way at the moment to get around the issue is to have the component export meta. I'd be happy to provide input on that layouts doc.

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

Successfully merging this pull request may close these issues.

7 participants