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

Fix issue with sb init on the Vue CLI #4535

Merged
merged 2 commits into from Oct 25, 2018
Merged

Conversation

tmeasday
Copy link
Member

Issue: #4475

What I did

Re add babel-preset-vue to vue init script.

Also install a manual dep on @babel/core, to fix yarn problems.

How to test

cd lib/cli
yarn link

cd /tmp
npx -p @vue/cli vue create new-app

cd new-app
sb init

yarn storybook

This was removed in e496dfb

Probably we should not even be using this preset (see #4475 (comment)), but we are, so I will add this for now to get it working.
@tmeasday tmeasday changed the title Tmeasday/fix sb init vue cli Fix issue with sb init on the Vue CLI Oct 24, 2018
@tmeasday tmeasday added maintenance User-facing maintenance tasks vue labels Oct 24, 2018
@tmeasday tmeasday requested a review from igor-dv October 24, 2018 05:22
// installBabel below. For some reason it leads to the wrong version of @babel/core (a beta)
// being installed
if (packageBabelCoreVersion === '7.0.0-bridge.0') {
addToDevDependenciesIfNotPresent(packageJson, '@babel/core', babelCoreVersion);
Copy link
Member

Choose a reason for hiding this comment

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

So this leads to both babel-core and @babel/core being installed?

Copy link
Member

Choose a reason for hiding this comment

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

Where the vue app uses babel-core and storybook uses @babel/core?

Copy link
Member

Choose a reason for hiding this comment

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

Is this problematic? I don't know enough to understand the implications here.

Copy link
Member Author

Choose a reason for hiding this comment

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

babel-core@7.0.0-bridge.0 is just a shim in front of @babel/core.

I don't really know why the Vue CLI installs it as it appears to use babel7.

Copy link
Member Author

Choose a reason for hiding this comment

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

The issue (seems to my limited eyes) to be that babel-core@7.0.0-bridge.0 depends on @babel/core@7.0.0-0 (note the -0), which leads to this weird beta version of @babel/core being installed. This overrides that.

I feel like this is a mistake in the vue CLI 🤷‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

Cool, that maybe makes sense.

@codecov
Copy link

codecov bot commented Oct 24, 2018

Codecov Report

Merging #4535 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4535      +/-   ##
==========================================
- Coverage   35.58%   35.57%   -0.02%     
==========================================
  Files         557      557              
  Lines        6730     6732       +2     
  Branches      883      884       +1     
==========================================
  Hits         2395     2395              
- Misses       3876     3877       +1     
- Partials      459      460       +1
Impacted Files Coverage Δ
lib/cli/lib/helpers.js 0.96% <0%> (-0.02%) ⬇️

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 5b2d316...9715b34. Read the comment docs.

@shilman shilman merged commit e2a83b0 into master Oct 25, 2018
@shilman shilman deleted the tmeasday/fix-sb-init-vue-cli branch October 25, 2018 03:33
@tmeasday
Copy link
Member Author

Confirmed fixed in rc.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks vue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants