Skip to content

Core: Group hooks in a single folder#31839

Merged
valentinpalkovic merged 1 commit into
nextfrom
sidnioulz/regroup-hooks
Jul 14, 2025
Merged

Core: Group hooks in a single folder#31839
valentinpalkovic merged 1 commit into
nextfrom
sidnioulz/regroup-hooks

Conversation

@Sidnioulz

@Sidnioulz Sidnioulz commented Jun 22, 2025

Copy link
Copy Markdown
Contributor

What I did

Grouped up hooks in a single folder in core/src/manager/hooks. One hook was previously all on its own in core/src/manager/components/hooks. Just a basic refactor PR.

Checklist for Contributors

Testing

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

N/A (covered by existing tests for layout)

Manual testing

  1. Run yarn task compile, notice it passes

Documentation

N/A

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

Relocated useMediaQuery hook from core/src/manager/components/hooks to core/src/manager/hooks to improve code organization by consolidating all hooks in a single directory.

  • Moved useMedia.tsx from components/hooks to dedicated hooks directory
  • Updated import path in LayoutProvider.tsx to reflect new hook location
  • No functional changes made to the hook implementation
  • Maintains existing test coverage through layout tests
  • Successful manual verification with yarn task compile

@Sidnioulz Sidnioulz added cleanup Minor cleanup style change that won't show up in release changelog ci:daily Run the CI jobs that normally run in the daily job. labels Jun 22, 2025
@Sidnioulz Sidnioulz requested a review from Copilot June 22, 2025 09:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors hook imports by grouping them into a single folder under "core/src/hooks".

  • Updated the import path for the useMediaQuery hook in LayoutProvider.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

2 files reviewed, no comments
Edit PR Review Bot Settings | Greptile

@nx-cloud

nx-cloud Bot commented Jun 22, 2025

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit d4d5f97.

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 1m 15s View ↗

☁️ Nx Cloud last updated this comment at 2025-06-22 10:22:45 UTC

@storybook-app-bot

Copy link
Copy Markdown

Package Benchmarks

Commit: d4d5f97, ran on 22 June 2025 at 10:11:51 UTC

The following packages have significant changes to their size or dependencies:

storybook

Before After Difference
Dependency count 49 49 0
Self size 31.85 MB 31.87 MB 🚨 +15 KB 🚨
Dependency size 17.41 MB 17.41 MB 0 B
Bundle Size Analyzer Link Link

sb

Before After Difference
Dependency count 50 50 0
Self size 1 KB 1 KB 0 B
Dependency size 49.26 MB 49.27 MB 🚨 +15 KB 🚨
Bundle Size Analyzer Link Link

@storybook/cli

Before After Difference
Dependency count 216 216 0
Self size 582 KB 582 KB 0 B
Dependency size 94.54 MB 94.55 MB 🚨 +15 KB 🚨
Bundle Size Analyzer Link Link

@storybook/codemod

Before After Difference
Dependency count 185 185 0
Self size 31 KB 31 KB 0 B
Dependency size 78.67 MB 78.69 MB 🚨 +15 KB 🚨
Bundle Size Analyzer Link Link

@Sidnioulz

Sidnioulz commented Jun 22, 2025

Copy link
Copy Markdown
Contributor Author

Judging from the size changes, should I instead move all hooks to src/manager/components/?

@valentinpalkovic

Copy link
Copy Markdown
Contributor

cc @ndelangen Can you answer the question of why we have two different hook folders inside of manager?

@valentinpalkovic valentinpalkovic merged commit 8431def into next Jul 14, 2025
97 of 99 checks passed
@valentinpalkovic valentinpalkovic deleted the sidnioulz/regroup-hooks branch July 14, 2025 13:22
@github-actions github-actions Bot mentioned this pull request Jul 14, 2025
18 tasks
@yannbf yannbf removed the Stale label Jul 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:daily Run the CI jobs that normally run in the daily job. cleanup Minor cleanup style change that won't show up in release changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants