-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Build: Fix Angular sandbox #23896
Build: Fix Angular sandbox #23896
Conversation
Updated and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: @storybook/client-logger@7.6.0, @storybook/theming@7.6.0 |
Thanks for the PR @Marklb ! 💪 How would I reproduce the issue that you're fixing here? |
Just cloning the repo, without these changes, and running any of the three Angular sandboxes has been failing for a while. |
I will update this to the current This only happens when using the dev build of the libraries, so I am applying them and keeping them unstaged every time I want to try any changes in an Angular sandbox. I can maybe add a condition to only apply the changes when generating a dev sandbox, if that is exposed to the sandbox generation, or do the libraries dev builds need to be changed? I don't remember which libraries were causing the issues, but I can try to find those instead of adjusting the sandbox. |
Hey Mark! Sorry we haven't looked into this yet. I'll do this as first thing next week. Thank you so much!! |
Hey @Marklb I updated the branch with the latest changes in 1:
2:
3:
4:
can you take a look into that? |
@yannbf 2: This why I regenerated the lock file, so I have been regenerating the lock file when I merge 3: I haven't seen this before, but I will check. More strict typescript rules may have introduced this. I have seen it in an Angular project, but based on the sandbox's tsconfig I don't see why it would happen. This is new, since I started this PR. 4: This a warning that I have seen off and on for years, but I don't know what is causing it. Stricter typescript rules may have made it an error. Even with the errors from 3 and 4, the sandbox seems to still work. |
Hey @Marklb, Thank you for your contribution! This PR is superseded by #24367. I have updated the yarn.lock file once again to fix an issue related to the Angular 15 sandbox on our CI. Another issue popped up and it seems, that your fix isn't needed anymore. I am closing this PR for now. Let me know if we should reopen it. |
It would be nice to not have to alter the default config, but reverting #24373 and applying the three If we want to reopen this PR, I can update this branch. |
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.
LGTM!
Closes #
What I did
Fix errors preventing Angular sandboxes from running.
I regenerated the lock file, to fix an issue with browserlist, I think. The error was
Unknown version 115 of edge (While processing: "base$0$0")
.The linked dev lib builds do not work with the default Angular tsconfig, so I set:
I did not narrow down what import is causing the
node_modules/@storybook/angular/node_modules/@angular-devkit
to get used, instead ofnode_modules/@angular-devkit
, but for the Angular 15 sandbox I set a path in the tsconfig:How to test
yarn task --task dev --template angular-cli/15-ts
,yarn task --task dev --template angular-cli/default-ts
,yarn task --task dev --template angular-cli/prerelease
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", "build", "documentation", "maintenance", "dependencies", "other"]
🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/core
team here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>