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

Allow injecting Vue #61

Closed
wants to merge 8 commits into from
Closed

Allow injecting Vue #61

wants to merge 8 commits into from

Conversation

fjc0k
Copy link
Contributor

@fjc0k fjc0k commented Apr 15, 2018

This PR is helpful for component developers to show their components, e.g.

.vuepress/inject.js:

import FirUI from '../../dist/fir-ui.min.js'
import '../../dist/fir-ui.min.css'

export default Vue => {
  Vue.use(FirUI)
}

components/button.md:

<f-button type="primary">Button</f-button>

@ulivz
Copy link
Member

ulivz commented Apr 16, 2018

Since the new version has supported to eject the default theme (89538fa) which can let you to mutate the Vue constructor, so I'm not sure whether it's necessary. Maybe this PR is good for those users who don't want to eject theme but still want to extend Vue.

@fjc0k
Copy link
Contributor Author

fjc0k commented Apr 16, 2018

@ulivz The new feat EJECT is good to develop a new theme. But most of the time, we just need to extend Vue. What's more, we want keep it independent on THE THEME.

lib/prepare.js Outdated
`inject(Vue)`
)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe adding a webpack alias to this file and import it in lib/app/app.js would be better

Copy link
Member

@ulivz ulivz Apr 16, 2018

Choose a reason for hiding this comment

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

IMO, This solution would be better.

@egoist
Copy link
Contributor

egoist commented Apr 16, 2018

And there's no need to inject Vue as the argument since you can directly import it, something else like app instance may be be more useful.

@fjc0k
Copy link
Contributor Author

fjc0k commented Apr 16, 2018

#80

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.

6 participants