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

Make vue a peer dependency #2041

Merged
merged 5 commits into from
Oct 20, 2017
Merged

Make vue a peer dependency #2041

merged 5 commits into from
Oct 20, 2017

Conversation

znck
Copy link
Contributor

@znck znck commented Oct 14, 2017

What I did

Depend loosely on vue & vue-loader by moving them to peer dependencies. This allows end user to
pick any version of vue for their project.

How to test

Existing tests should pass.

Depend loosely on vue & vue-loader by moving them to peer dependencies.
Copy link
Member

@kazupon kazupon left a comment

Choose a reason for hiding this comment

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

Thanks!

@Hypnosphi
Copy link
Member

@kazupon Is there a way we can merge this PR with #2032?

@kazupon
Copy link
Member

kazupon commented Oct 15, 2017

@Hypnosphi Once merge #2032 to master branch, after that pull from this master branch to this PR by [Update branch] of GitHub's WebUI. How is a way?

@Hypnosphi
Copy link
Member

I mean, that one needs review, but it partially interleaves with this one, so I'm not sure what should we do

"vue-template-compiler": "^2.5.2",
},
"peerDependencies": {
"vue": "*",
Copy link
Member

Choose a reason for hiding this comment

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

Can we check that we work with any vue version?

Copy link
Member

Choose a reason for hiding this comment

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

I think that we should be checked vue latest version only. because vue-template-compiler forces the same version as vue

Copy link
Member

Choose a reason for hiding this comment

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

I mean, do we work with vue version 1? If not, we shouldn't put an asterisk here

"nodemon": "^1.12.1",
"vue": "^2.5.2",
"vue-loader": "^13.3.0",
"vue-template-compiler": "^2.5.2",
Copy link
Member

Choose a reason for hiding this comment

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

Trailing comma makes JSON invalid

@Hypnosphi Hypnosphi changed the title bump vue version Make vue a peer dependency Oct 16, 2017
"vue": "^2.5.2",
"vue-hot-reload-api": "^2.2.0",
"vue-loader": "^12.2.1",
"vue-hot-reload-api": "^2.1.1",
Copy link
Member

Choose a reason for hiding this comment

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

It should be 2.2.0

Copy link
Member

Choose a reason for hiding this comment

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

Please run yarn install to update lockfile after you fix it

Copy link
Member

Choose a reason for hiding this comment

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

Not a deal-breaking for this PR, TBH.

"nodemon": "^1.12.1"
"nodemon": "^1.12.1",
"vue": "^2.5.2",
"vue-loader": "^13.3.0",
Copy link
Member

Choose a reason for hiding this comment

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

What are the breaking changes between 12 and 13?

Choose a reason for hiding this comment

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

@Hypnosphi https://github.com/vuejs/vue-loader/releases/tag/v13.0.0

Upgrading vue-loader also fixes vuejs/vue-loader#951 (supporting nested css rules). Which is something I use heavily in my .vue files and that's currently blocking me from adding several components to my storybook.

So would love to see this getting merged eventually!

Copy link
Member

Choose a reason for hiding this comment

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

@codecov
Copy link

codecov bot commented Oct 17, 2017

Codecov Report

Merging #2041 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2041   +/-   ##
======================================
  Coverage    21.4%   21.4%           
======================================
  Files         263     263           
  Lines        5793    5793           
  Branches      695     694    -1     
======================================
  Hits         1240    1240           
- Misses       4009    4029   +20     
+ Partials      544     524   -20
Impacted Files Coverage Δ
app/react/src/server/config/babel.js 0% <0%> (-100%) ⬇️
app/react/src/server/babel_config.js 0% <0%> (-77.42%) ⬇️
app/vue/src/server/utils.js 0% <0%> (-53.58%) ⬇️
addons/knobs/src/components/PropField.js 10.63% <0%> (ø) ⬆️
addons/knobs/src/components/types/Select.js 7.93% <0%> (ø) ⬆️
app/react-native/src/bin/storybook-build.js 0% <0%> (ø) ⬆️
addons/knobs/src/KnobStore.js 9.09% <0%> (ø) ⬆️
addons/knobs/src/components/types/Object.js 5.81% <0%> (ø) ⬆️
lib/ui/src/modules/ui/configs/init_panels.js 25% <0%> (ø) ⬆️
...es__/update-addon-info/update-addon-info.output.js 0% <0%> (ø) ⬆️
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9ca582...18d5528. Read the comment docs.

"vue": "^2.5.2",
"vue-hot-reload-api": "^2.2.0",
"vue-loader": "^12.2.1",
"vue-hot-reload-api": "^2.1.1",
Copy link
Member

Choose a reason for hiding this comment

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

Not a deal-breaking for this PR, TBH.

"nodemon": "^1.12.1"
"nodemon": "^1.12.1",
"vue": "^2.5.2",
"vue-loader": "^13.3.0",
Copy link
Member

Choose a reason for hiding this comment

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

@ndelangen
Copy link
Member

@Hypnosphi Looks like you have no huge objections to this PR, so I hope you don't mind me merging this as Vue-version mismatches are a huge problem for users.

@ndelangen ndelangen merged commit 334bfa4 into storybookjs:master Oct 20, 2017
@ndelangen
Copy link
Member

Thank you for your efforts @znck @kazupon @eriknygren @Hypnosphi 👍 💯

@Hypnosphi
Copy link
Member

Hypnosphi commented Oct 20, 2017

Actually, I had one, about the asterisk: #2041 (comment)

@znck znck deleted the patch-1 branch October 21, 2017 04:38
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.

5 participants