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

feat: allow computed properties within useMeta #255

Merged
merged 3 commits into from
Oct 12, 2020

Conversation

danielroe
Copy link
Member

@danielroe danielroe commented Oct 9, 2020

Usage:
In addition to the existing usage for useMeta, this adds the possibility to pass a function which will effectively generate a computed head state and be merged with any other head values.

useMeta(() => ({ title: message.value }))

nuxt-community/typescript-template#225

closes #254

@danielroe danielroe self-assigned this Oct 9, 2020
@vercel
Copy link

vercel bot commented Oct 9, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nuxt-community/composition-api/hyqyi8bk6
✅ Preview: https://composition-api-git-254-allow-computed-meta.nuxt-community.vercel.app

@danielroe
Copy link
Member Author

@mathe42 I've reverted much of the special handling of titleTemplate as it seems to work now with an initial null value - but want to check there are no gotchas I'm unaware of. Would value your thoughts.

@mathe42
Copy link
Collaborator

mathe42 commented Oct 12, 2020

When null doesn't overwrite the global meta data it's fine.

I can't test this in the next 2 weeks

@danielroe danielroe merged commit 7152ed4 into main Oct 12, 2020
@danielroe danielroe deleted the 254-allow-computed-meta branch October 12, 2020 07:23
if (process.client)
watch(Object.values(refs), vm.$meta().refresh, { immediate: true })
if (init instanceof Function) {
_computedHead.push(computed(init))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible that there is a bug here?

Is it possible that pushing something to computedHead add the computedRef at component level instead of instance level?

I am trying to debug a problem where a meta tag is repeated use the function syntax useMeta(() => ({ ... }))

Copy link
Member Author

@danielroe danielroe Oct 13, 2020

Choose a reason for hiding this comment

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

Have you added the hid property to deduplicate?

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right, I am not adding the property hid, but I think that the property only will hide the problem.

The problem is not related to several components trying to write the same property. I only have one component on the tree trying using useMeta

 setup() {
    useMeta(() => {
      return {
        meta: [
          { property: 'og:title', content: 'page title' }
        ]
      }
    });
  }

The problem I am facing is that every request is adding a new duplicated property (I am doing server-side rendering). So I think that somehow, the state is being stored between each request.

If I print the value _computedHead here https://github.com/nuxt-community/composition-api/pull/255/files#diff-e0058ab35c645c7435fbc38c50432eabeec5d30d5ad15c4278e083edb897842dR106 I can see that it add a new entry on each request.

I will try to create a minimal reproduction repository.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @axelhzf - good catch.

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.

feat: computed properties within useMeta
3 participants