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

Controls: Fix esm is not defined error with built Storybook #15812

Merged
merged 1 commit into from
Aug 12, 2021

Conversation

gaetanmaisse
Copy link
Member

Issue: #15773

What I did

I reverted #14394 because it looks like it's the commits that introduced the esm is not defined error. It will need some deeper investigation.

How to test

  • e2e ci steps should be back to 🟢

@nx-cloud
Copy link

nx-cloud bot commented Aug 10, 2021

Nx Cloud Report

CI ran the following commands for commit 5f589a2. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

@gaetanmaisse gaetanmaisse added the maintenance User-facing maintenance tasks label Aug 10, 2021
@gaetanmaisse
Copy link
Member Author

Angular related E2E tests are still failing but for another reason, addon-controls doesn't consider user as an arg 🤔 (with a def value)

image

@gaetanmaisse
Copy link
Member Author

@shilman @stevensacks was the unbox the value to simplify the code commit mainly cosmetic or was there a deep reason for that changes?

@shilman
Copy link
Member

shilman commented Aug 11, 2021

@gaetanmaisse the old boxing/unboxing code was a hack because when you try to use React's useState with a function value it breaks unless you either (1) box it, or (2) do what @stevensacks proposed which is simpler and slightly more efficient. However, until we have a fix for the problem at hand (which maybe has something to do with how the fix interacts with our webpack settings? or maybe just isn't quite right?) we should revert since the bug is a blocker and getting in the way of people using the alphas.

@gaetanmaisse gaetanmaisse marked this pull request as ready for review August 12, 2021 13:32
@gaetanmaisse gaetanmaisse requested a review from a team August 12, 2021 13:34
@gaetanmaisse gaetanmaisse changed the title Build: Fix addon-controls with built Storybook Controls: Fix esm is not defined error with built Storybook Aug 12, 2021
@gaetanmaisse gaetanmaisse merged commit c46dd2a into next Aug 12, 2021
@gaetanmaisse gaetanmaisse deleted the fix-addon-controls branch August 12, 2021 13:52
@shilman shilman added this to the 6.4 PRs milestone Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants