-
-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
[ember] add dependencies to root application #4309
[ember] add dependencies to root application #4309
Conversation
@@ -28,12 +28,15 @@ | |||
"@babel/runtime": "^7.0.0", | |||
"@ember/test-helpers": "^0.7.25", | |||
"@storybook/core": "4.0.0-alpha.24", | |||
"common-tags": "^1.8.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this moved?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is used in @storybook/ember and that's it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't ember itself depend on this dep?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, it is used for outputting error information in the render function
Generated by 🚫 dangerJS |
3020f1f
to
abe6d8f
Compare
Codecov Report
@@ Coverage Diff @@
## master #4309 +/- ##
=======================================
Coverage 40.23% 40.23%
=======================================
Files 510 510
Lines 5952 5952
Branches 795 795
=======================================
Hits 2395 2395
Misses 3172 3172
Partials 385 385 Continue to review full report at Codecov.
|
@igor-dv since adding react and react-dom to my app dependencies it is now being recognised as a react app by Does the order of if statements in the app detection need reworking so ember is detected before react? |
@Keraito is working on a CLI improvement where you will be able to pass a flag of the wanted framework. Probably |
yes, it will probably help, want to PR ?
I am less familiar with the CLI part. @Keraito @gabrielcsapo do you have something to say about this ? |
@igor-dv compiling PR now.
Having now added 'ember' to that list, it is correctly detecting when storybook is already setup so I think it needs to be in there and I'll add to my PR in a separate commit. |
Fixes to cli ember support (followup on #4309)
Issue: #4289
What I did
Add dependencies to the top level @storybook/ember application