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

Fix dependency on @storybook/node-logger and @storybook/client-logger #182

Merged
merged 4 commits into from
May 16, 2024

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented May 16, 2024

This fixes:

The core consolidation project will cause unresolved packages to start failing storybook.
This package has such unresolved dependencies.

Mind you this would have already broken pnp mode and possible other strict dependency managers.
So it's a thing we want to fix, even if the core package consolidation project is delayed or cancelled.

1 solution would be to move these to regular dependencies, instead of devDependencies.
But with the core package consolidation project in mind, removing/reducing the dependency on those packages is a good move.

What I did:

  • I updated some devDependencies, because why not?
  • I removed the need for @storybook/node-logger, I deemed it not important enough to keep.
  • I removed the use of @storybook/client-logger in the piece of code that's shared between server & browser.

The dependency on @storybook/node-logger was missing completely!

@storybook/client-logger was being referenced in a piece of code used by both node & browser.
This is fine for browser, because of prebundling (globalized in both preview & manager), but this is really bad for node, because:

  • the dependency is missing
  • it's loading browser code into node.

QA:

I've manually tested this by:

  • generating a svelte sandbox
  • changing the dependency version
  • installing
  • running build-storybook (was not working before this patch)
📦 Published PR as canary version: 4.1.3--canary.182.cb1f688.0

✨ Test out this PR locally via:

npm install @storybook/addon-svelte-csf@4.1.3--canary.182.cb1f688.0
# or 
yarn add @storybook/addon-svelte-csf@4.1.3--canary.182.cb1f688.0

@ndelangen ndelangen requested a review from JReinhold May 16, 2024 14:42
@ndelangen ndelangen self-assigned this May 16, 2024
@JReinhold JReinhold added patch Increment the patch version when merged dependencies Update one or more dependencies version labels May 16, 2024
Copy link
Collaborator

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

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

LGTM!

@JReinhold JReinhold changed the title remove the dependency on client-logger in shared node/browser code & updates Fix dependency on @storybook/node-logger and @storybook/client-logger May 16, 2024
@JReinhold JReinhold merged commit 14ec1cb into main May 16, 2024
4 checks passed
@shilman
Copy link
Member

shilman commented May 16, 2024

🚀 PR was released in v4.1.3 🚀

@shilman shilman added the released This issue/pull request has been released. label May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Update one or more dependencies version patch Increment the patch version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants