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

Core: Deprecate storyStoreV6 (including storiesOf) and storyIndexers #23938

Merged
merged 13 commits into from Oct 3, 2023

Conversation

JReinhold
Copy link
Contributor

@JReinhold JReinhold commented Aug 24, 2023

Works on #23457

image

image

What I did

  • Added deprecation warning for storyIndexers and for the corresponding types
  • Added migration notes for storyIndexers
  • Added deprecation warning and migration notes for storyStoreV6, explicitly calling out storiesOf

The deprecation warning for storyIndexers only appears when a deprecated indexer is handling a file, not just when it's configured. That way, addon authors can support both the old and new API by setting both properties and not get punished with a deprecation warning. In SB 7.5.0 storyIndexers will simply be ignored, while previous versions will ignore experimental_indexers.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

I tested this by adding features: { storyStoreV7: false } to the configuration and saw the deprecation warning in both dev and build. The warnings are not shown if it's true nor if it's unspecified.

I tested the storyIndexers deprecation warning by adding an indexer to the config and removing all experimental_indexers with:

// .storybook/main.ts

import { readFileSync } from 'fs';
import { loadCsf } from '@storybook/csf-tools';

export default {
  experimental_indexers: () => [],
  storyIndexers = (indexers) => [
    {
      test: /(stories|story)\.[tj]sx?$/,
      indexer: async (fileName, options) => (await readCsf(fileName, options)).parse(),
    },
    ...(indexers || []),
  ],
};

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 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>

@JReinhold JReinhold changed the title Deprecate storyStoreV6 (including storiesOf) and `storyIndexers Core: Deprecate storyStoreV6 (including storiesOf) and `storyIndexers Aug 24, 2023
@JReinhold JReinhold changed the title Core: Deprecate storyStoreV6 (including storiesOf) and `storyIndexers Core: Deprecate storyStoreV6 (including storiesOf) and `storyIndexers* Aug 24, 2023
@JReinhold JReinhold changed the title Core: Deprecate storyStoreV6 (including storiesOf) and `storyIndexers* Core: Deprecate storyStoreV6 (including storiesOf) and storyIndexers Aug 24, 2023
@JReinhold JReinhold marked this pull request as ready for review September 25, 2023 11:14
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

LGTM! Great stuff

Comment on lines +42 to +45
deprecate(
dedent`storyStoreV6 is deprecated, please migrate to storyStoreV7 instead.
- Refer to the migration guide at https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#storystorev6-and-storiesof-is-deprecated`
);
Copy link
Member

Choose a reason for hiding this comment

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

@yannbf I'd like to hear your opinion on if we should continue to do deprecations warnings like this -in place-?

I'd love to figure out a way to use the errors-consolidation work you've done, to consolidate deprecation warnings like these.

Copy link
Member

Choose a reason for hiding this comment

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

let's create a task for this!

@JReinhold JReinhold merged commit 3270b62 into next Oct 3, 2023
53 checks passed
@JReinhold JReinhold deleted the deprecate-storyindexers branch October 3, 2023 11:55
@github-actions github-actions bot mentioned this pull request Oct 3, 2023
20 tasks
@yannbf yannbf mentioned this pull request Oct 17, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:normal core maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants