Skip to content
This repository has been archived by the owner on Aug 20, 2024. It is now read-only.

fix: remove addons method for Storbook 5.x #20

Merged
merged 1 commit into from
Jan 23, 2020
Merged

Conversation

chrisabrams
Copy link
Contributor

@chrisabrams chrisabrams commented Jan 15, 2020

When running Storybook 5.3, I ran into an issue with this method being passed in where Storybook actually expects an array of presets. Removing this method enables Storybook to build and render as expected.

See the Storybook docs where addons was changed: https://storybook.js.org/docs/presets/introduction/#basic-usage

Based on what I'm seeing in the docs I'd assume this is a breaking change.

Copy link
Contributor

@msluther msluther left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like @indexzero to weigh in. I don't remember the exact issues around this. I know it was to try to dynamically add in ./examples/.setup/addons.js. Removing that is definitely a breaking change. And if we want to remove it, there's definitely other pieces that should get removed here as well.

@chrisabrams
Copy link
Contributor Author

Storybook no longer expects an addons.js file. Now it just expects addons as a property in main.js.

Copy link
Contributor

@indexzero indexzero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍reading the latest Storybook docs on writing presets addons appears to be an Array, not a function that returns an Array.

Not sure where that changed, but if this is released as a major semver bump it will cover any unexpected breaking changes to consumers.

@msluther
Copy link
Contributor

5.2.0 -> 5.3.0 was a breaking change in storybook: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#from-version-52x-to-53x

@chrisabrams
Copy link
Contributor Author

chrisabrams commented Jan 15, 2020

@indexzero it changed here: https://medium.com/storybookjs/declarative-storybook-configuration-49912f77b78

Which to them is not considered major.

@msluther
Copy link
Contributor

I'm okay with this PR, but we should probably publish as an alpha/beta until we get all the storybook@5.3 kinks worked out :(

@chrisabrams
Copy link
Contributor Author

FYI they did get back to me on this error: storybookjs/storybook#9311

@chrisabrams chrisabrams merged commit c1357f5 into master Jan 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants