-
-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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 broken import #21435
Ember: Fix broken import #21435
Conversation
@@ -2,7 +2,7 @@ import { global } from '@storybook/global'; | |||
import { dedent } from 'ts-dedent'; | |||
import type { RenderContext } from '@storybook/types'; | |||
// @ts-expect-error (Converted from ts-ignore) |
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.
I guess we can remove this line types for @glimmer
should be defined
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.
Done
Changes look good to me, did you had the opportunity to try with yarn link or something like this? The CI was already not breaking last time and I've introduced the bug so I better be extra careful :) |
e06da68
to
5caecd8
Compare
Actually we tried, but I'm not sure on how to fully test this. Here are the steps I followed: Build and link
|
I believe this one happens because my local fork is Storybook@v7, so |
I think you would need to try the fix on 6.15 version instead of the |
So I have no idea on how to fix this. The Ember example throws a lot of warnings. Upgrading to new versions totally breaks and we're not even in the latest major one. If I understand it correctly, Ember team looks to know about it. I hope they can fix it. I'm closing my PR. |
Hopefully closes #20653
What I did
Ember recommends to use
@glimmer/component
rather than@ember/component
. I switched the import and the package.json accordingly. I believe ember itself somehow import@glimmer/component
so maybe the peer dependency is not needed. I couldn't launch the project myself locally so let's wait for CI check.How to test
Checklist
MIGRATION.MD
Maintainers
make sure to add the
ci:merged
orci:daily
GH label to it.["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]