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

enhanceApp options should merge options we pass #1631

Closed
flozero opened this issue May 31, 2019 · 14 comments
Closed

enhanceApp options should merge options we pass #1631

flozero opened this issue May 31, 2019 · 14 comments

Comments

@flozero
Copy link
Collaborator

flozero commented May 31, 2019

Feature request

actually i am building additional page from components with docs

https://github.com/f3ltron/vuepress-plugin-docgen

// https://github.com/vuejs/vuepress/blob/master/packages/%40vuepress/core/lib/client/app.js#L88

const options = {}

  try {
    appEnhancers.forEach(enhancer => {
      if (typeof enhancer === 'function') {
        enhancer({ Vue, options, router, siteData, isServer })
      }
    })
  } catch (e) {
    console.error(e)
  }

  const app = new Vue(
    Object.assign(options, {
      router,
      render (h) {
        return h('div', { attrs: { id: 'app' }}, [
          h('router-view', { ref: 'layout' }),
          h('div', { class: 'global-ui' }, globalUIComponents.map(component => h(component)))
        ])
      }
    })
  )

What problem does this feature solve?

when we use some front plugin like vue18n in my case by letting me merge my options into options there will be no more bug for $t is not define

What does the proposed API look like?

The merge should merge dynamically our options

Are you willing to work on this yourself?

i can if it's accepted

@ulivz
Copy link
Member

ulivz commented Jul 29, 2019

Sorry for the delay to see this pull request, do you still need it?

@flozero
Copy link
Collaborator Author

flozero commented Jul 29, 2019

no problem yeah it could be great as the feature for me should exist when i watch the code. Maybe i am wrong ?

@MickaMx
Copy link

MickaMx commented Aug 2, 2019

I'd be great to add that. I've been trying to import my vuetify component to vuepress but those components use vue-i18n and I can't find a way to make it work.
This feature could solve the problem

@flozero
Copy link
Collaborator Author

flozero commented Aug 2, 2019

If i remember well it was exactly for that i created the issue because i faced the same probleme @MickaMx :)

@MickaMx
Copy link

MickaMx commented Aug 2, 2019

Did you found a way around ? other than the code you've add in this issue?

@flozero
Copy link
Collaborator Author

flozero commented Aug 2, 2019

This part of code should be update. I could create the PR but just need some validation on it

@flozero
Copy link
Collaborator Author

flozero commented Aug 2, 2019

yeah i found something but really not stable

@flozero
Copy link
Collaborator Author

flozero commented Aug 2, 2019

oh didnt remember ^ ^ i already created the pr @MickaMx #1632

@MickaMx
Copy link

MickaMx commented Aug 2, 2019

Yep, I saw that. It didn't got merge yet ^^

@beatgrabe
Copy link

Wow. I have exactly the same problem now. I am really looking forward to have this merged.

@flozero
Copy link
Collaborator Author

flozero commented Sep 17, 2019

We will publish it the next release

@meteorlxy
Copy link
Member

meteorlxy commented Sep 21, 2019

What's the problem if we use Object.assign(options, {}) in current enhanceApp ?

// .vuepress/enhanceApp.js
import Vuex from 'vuex'

export default ({ Vue, options }) => {
  Vue.use(Vuex)

  const store = new Vuex.Store({
    state: {
      msg: 'hello'
    },
  })

  Object.assign(options, {
    store
  })
}
// README.md

{{ $store.state.msg }}

That works well.

🤔 So what's this FR for? Sorry if I misunderstood this FR.

@flozero
Copy link
Collaborator Author

flozero commented Sep 22, 2019

@meteorlxy it change the option by reference ?

@flozero
Copy link
Collaborator Author

flozero commented Jan 28, 2020

@meteorlxy answered to the question thx i am closing the issue

@flozero flozero closed this as completed Jan 28, 2020
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 a pull request may close this issue.

5 participants