-
-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
Maintenance: Move certain internal entries to top-level-export #30794
base: valentin/move-addon-actions-into-core
Are you sure you want to change the base?
Maintenance: Move certain internal entries to top-level-export #30794
Conversation
…t package imports
…ct package imports
…ct package imports
… direct package import
…t package imports
… direct package import
… to direct package import
…direct package import
… direct package imports
View your CI Pipeline Execution ↗ for commit a2246f9.
☁️ Nx Cloud last updated this comment at |
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.
385 file(s) reviewed, 8 comment(s)
Edit PR Review Bot Settings | Greptile
import { IconButton } from 'storybook/internal/components'; | ||
import { useGlobals, useParameter } from 'storybook/internal/manager-api'; | ||
|
||
import { GridIcon } from '@storybook/icons'; | ||
|
||
import { useGlobals, useParameter } from 'storybook/manager-api'; |
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.
style: Inconsistent import organization - internal components import should be grouped with manager-api import since they're both from storybook
const regexp = /'lib\/preview-api/; | ||
const replaced = content.replace(regexp, "'storybook/internal/preview-api"); | ||
const replaced = content.replace(regexp, "'storybook/preview-api"); |
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.
style: Consider using a more specific regex pattern like /'lib/preview-api'/ to avoid potential false matches
@@ -20,7 +20,7 @@ Even though this is where all of the code is located, it is NOT to be the entry | |||
Consumers of the code should import like so: | |||
|
|||
```ts | |||
import { addons } from 'storybook/internal/manager-api'; | |||
import { addons } from 'storybook/manager-api'; |
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.
logic: The import path has been updated from 'storybook/internal/manager-api' to 'storybook/manager-api', which appears to conflict with the guidance in line 41 that states packages should import from 'storybook/internal'
code/core/scripts/prep.ts
Outdated
'storybook/preview-api': join(cwd, 'src', 'preview-api'), | ||
'storybook/manager-api': join(cwd, 'src', 'manager-api'), | ||
'storybook/theming': join(cwd, 'src', 'theming'), | ||
'storybook/test': join(cwd, 'src', 'test'), | ||
|
||
'storybook/internal': join(cwd, 'src'), | ||
'storybook/actions': join(cwd, 'src', 'actions'), |
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.
style: Duplicate alias definitions between bundles and finals sections. Consider extracting common aliases into a shared configuration object to avoid duplication and potential inconsistencies
@@ -1,6 +1,6 @@ | |||
import React from 'react'; | |||
|
|||
import { ignoreSsrWarning, styled } from 'storybook/internal/theming'; | |||
import { ignoreSsrWarning, styled } from 'storybook/theming'; | |||
|
|||
const toNumber = (input: any) => (typeof input === 'number' ? input : Number(input)); |
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.
style: Consider adding type safety by explicitly typing the return value as number
import type { API_Provider } from 'storybook/internal/types'; | ||
|
||
import EventEmitter from 'events'; | ||
import { themes } from 'storybook/theming'; |
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.
style: Import order should be consistent - external imports should be grouped together before internal imports. Move EventEmitter import up with other external imports.
import { IconButton, Separator } from 'storybook/internal/components'; | ||
import { types } from 'storybook/internal/manager-api'; | ||
import type { Addon_BaseType } from 'storybook/internal/types'; | ||
|
||
import { ZoomIcon, ZoomOutIcon, ZoomResetIcon } from '@storybook/icons'; | ||
|
||
import { types } from 'storybook/manager-api'; |
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.
style: Imports are not consistently ordered - consider grouping external imports first, then internal Storybook imports, then local imports
import { SNIPPET_RENDERED, SourceType } from 'storybook/internal/docs-tools'; | ||
import { addons, useEffect } from 'storybook/internal/preview-api'; | ||
import type { DecoratorFunction } from 'storybook/internal/types'; | ||
|
||
import { addons, useEffect } from 'storybook/preview-api'; |
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.
style: Consider grouping related imports together - both SNIPPET_RENDERED
and addons
are from the core Storybook API
This commit updates import paths for manager-api and theming modules across multiple manager components, replacing 'storybook/internal/manager-api' and 'storybook/internal/theming' with 'storybook/manager-api' and 'storybook/theming' respectively.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
Before | After | Difference | |
---|---|---|---|
Dependency count | 102 | 102 | 0 |
Self size | 21.00 MB | 22.05 MB | 🚨 +1.05 MB 🚨 |
Dependency size | 24.62 MB | 24.62 MB | 0 B |
Bundle Size Analyzer | Link | Link |
sb
Before | After | Difference | |
---|---|---|---|
Dependency count | 103 | 103 | 0 |
Self size | 1 KB | 1 KB | 0 B |
Dependency size | 45.62 MB | 46.67 MB | 🚨 +1.05 MB 🚨 |
Bundle Size Analyzer | Link | Link |
@storybook/cli
Before | After | Difference | |
---|---|---|---|
Dependency count | 392 | 392 | 0 |
Self size | 284 KB | 284 KB | 🎉 -45 B 🎉 |
Dependency size | 95.59 MB | 96.63 MB | 🚨 +1.05 MB 🚨 |
Bundle Size Analyzer | Link | Link |
@storybook/codemod
Before | After | Difference | |
---|---|---|---|
Dependency count | 313 | 313 | 0 |
Self size | 31 KB | 31 KB | 0 B |
Dependency size | 77.38 MB | 78.43 MB | 🚨 +1.05 MB 🚨 |
Bundle Size Analyzer | Link | Link |
This commit adjusts import paths for actions and preview modules across multiple files, including: - Updating the import path for definePreview in preview and test modules - Modifying the import path for action annotations in core-annotations - Removing some unused exports from the manager globals - Adjusting the mapping of 'storybook/actions' in the prep script
This commit updates the globals and exports for Storybook modules, including: - Removing deprecated action-related exports - Adding deprecated internal module mappings - Reorganizing the order of module exports and globals - Preparing for future module consolidation and cleanup
This commit removes deprecated action-related module exports from the globals map, including: - Removing 'storybook/actions/preview', 'storybook/actions/manager', and 'storybook/actions/decorator' exports - Reorganizing the order of module exports - Adding a deprecated TODO comment for 'storybook/internal/preview-api'
What I did
This pull request primarily focuses on updating import paths for various Storybook modules to improve code organization and maintainability. The changes involve moving from internal paths to more standardized ones. Here are the most important changes:
Import Path Updates:
manager-api
incode/.storybook/main.ts
tostorybook/manager-api
.manager-api
incode/.storybook/manager.tsx
.code/.storybook/preview.tsx
to usestorybook/preview-api
andstorybook/theming
. [1] [2]preview-api
incode/addons/a11y/src/a11yRunner.test.ts
andcode/addons/a11y/src/a11yRunner.ts
. [1] [2]theming
in multiple files withincode/addons/a11y/src/components/
. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]These changes ensure that the codebase uses consistent and updated paths for Storybook modules, improving maintainability and readability.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
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-storybook/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>
Greptile Summary
This PR updates import paths across the Storybook codebase, moving several core modules from internal paths to top-level exports for better organization and maintainability.
storybook/internal/manager-api
tostorybook/manager-api
storybook/internal/preview-api
tostorybook/preview-api
storybook/internal/theming
tostorybook/theming
The changes are purely organizational and don't affect functionality, making core APIs more accessible while maintaining backward compatibility.