-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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: Fix @storybook/ember #23435
Ember: Fix @storybook/ember #23435
Conversation
5384547
to
6bfb7c0
Compare
12c1990
to
ab9606d
Compare
With storybookjs/ember-cli-storybook#183, I updated the sandbox template to ember 4.12 and it worked. |
@francois2metz Thank you for putting the effort here 🙏 . Looking forward to this PR to be merged 👍 |
Small bump to get this merged soon! 🙏 Thanks for contributing @francois2metz ! |
Hi @sebasjimenez10, @esbanarango and thanks! Could you test to see if it's working for you? |
I did try locally from a fresh storybook clone with Node18 and Ember4. I followed the steps described in the description. A few notes:
|
I retried from a fresh clone, using Node16 and got the same results. |
1e9eee0
to
8a5401f
Compare
The code changes relating to the core & sandboxes all look 👍 to me. I can't really say anything meaningful about the ember changes. |
We actually got it working on https://ui.pix.fr using https://github.com/1024pix/pix-ui/blob/928360355e9b4a63a48a42606373af66d01208b8/package.json#L62 :) |
8a5401f
to
01656ce
Compare
061f87d
to
c3d59e5
Compare
@francois2metz I got CI green in #25162. If you can update this PR with those changes, we should be able to get this one green |
I fixed the template names and it worked. Nothing that I can understand. |
It's greeeeeeeeeeeeeeeeeeeen 🥳 🎉 |
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.
Great job @francois2metz !! Thanks so much for bearing with us on this one 😅
thanks @shilman. I would love to backport this PR on the 7.x branch. What do you think? |
@shilman is it not possible to get this backported to 7.x? I'm assuming a stable 8 release is still quite a while away. |
Yeah, a backport to v7 sounds nice and we can have a stable sb@v7 with an stable ember version. But also:
I think it makes more sense to improve the DX for working with sb@8 at ember and aim at for a good experience once that hits a final release (given there is low capa from ember side, this might be converging on either schedules). That is a very sober view on the practical side of things, which I see very realistic - I'm still with you on how that looks and rest assured my perfectionistic artistic sense is hammering this message into me, too 😬 |
@chasegiunta we typically only patch very small, non-disruptive changes and I would say this doesn't qualify. Fortunately, we're shooting for an 8.0 release at end of Feb, so it isn't too far out at this point. |
Closes #20653
What I did
This PR fixes the ember addon on the latest 7.0 release. It was also broken on the 6.0.0 release since #17843.
The code has been updated to install ember with the webpack builder and the patch #17843 has been reverted.
To make thinks easier to test, I added two sandbox templates for ember 3 and ember 4.
How to test
Well, that's the fun part
Ember 3
yarn task --task dev --template ember/3-js --start-from=install
sandbox/ember-3-js
ember-auto-import
to^2.0.0
.ember-qunit
to^6.0.0
.yarn
yarn add @storybook/ember-cli-storybook
yarn build && yarn storybook
Ember 4
yarn task --task dev --template ember/default-js --start-from=install
Checklist
MIGRATION.MD
Maintainers
ci:normal
,ci:merged
orci:daily
GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli/src/sandbox-templates.ts
["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]
🦋 Canary release
This pull request has been released as version
0.0.0-pr-23435-sha-bd98198e
. Install it by pinning all your Storybook dependencies to that version.More information
0.0.0-pr-23435-sha-bd98198e
fix-ember-3.28
bd98198e
1692544911
)To request a new release of this pull request, mention the
@storybookjs/core
team.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=23435