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

updating lib/csf-tools to use ts-up #18914

Merged

Conversation

josh-the-dev
Copy link
Contributor

Issue:

What I did

Migrated csf-tools to use the ts-up bundler.

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

@josh-the-dev
Copy link
Contributor Author

Not sure why my tests are failing? If anycone can shed any light that'd be appreciated!

@MichaelArestad MichaelArestad added maintenance User-facing maintenance tasks core labels Aug 15, 2022
@ndelangen
Copy link
Member

I tried looking into this, why the tests are failing.. something is doing something funny with csf-tools that makes it not accept the bundled code. Perhaps it's some ESM detection thing. It seems like the test are detecting an actual regression though.

@josh-the-dev
Copy link
Contributor Author

I tried looking into this, why the tests are failing.. something is doing something funny with csf-tools that makes it not accept the bundled code. Perhaps it's some ESM detection thing. It seems like the test are detecting an actual regression though.

Not sure how I've managed to cause a regression through this though if you look at my changes? 😢 I've applied the changes as per other PRs and as the suggestion in the original issue post 👍 .
I'm more than welcome to feedback if I have broken something as a side effect though!

@ndelangen
Copy link
Member

Looks like we got a duplicate PR here:
#19141

In which I found that minification actually breaks to code 😱

@IanVS
Copy link
Member

IanVS commented Sep 13, 2022

Oh sorry, I didn't notice this PR when I made mine, it wasn't linked back to #18732 I guess.

@ndelangen ndelangen merged commit 78db6ee into storybookjs:next Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants