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 prepare bundle on Windows #19243

Merged

Conversation

joshwooding
Copy link
Member

@joshwooding joshwooding commented Sep 24, 2022

Issue:

image

What I did

Stopped the entries being passed into tsup being made into absolute paths.

This fixed things for me locally but looking at the tsup docs entry always contains relative files:

image

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.

@joshwooding joshwooding force-pushed the fix-prepare-bundle-on-windows branch from bdf8ee7 to 7bf87d3 Compare September 24, 2022 13:32
@ndelangen
Copy link
Member

@joshwooding thank you for the contribution!

Why are absolute paths a problem? We're using path.join which should yield a proper absolute path on either platform?

Might be a bug in tsup rather.?

Because of the fact we're running these scripts in multiple working directories, it's easy to get mixed up.
Having absolute paths can remove some confusion.
I'd rather keep it and fix this upstream.

@joshwooding joshwooding force-pushed the fix-prepare-bundle-on-windows branch from 7bf87d3 to 6cc0971 Compare September 27, 2022 22:26
@joshwooding
Copy link
Member Author

Thanks @ndelangen after some digging an alternative is to replace the windows path separators with Linux ones. Arguably the fix should belong in tsup but making this change unblocks us for now.

@ndelangen ndelangen self-assigned this Sep 28, 2022
@ndelangen ndelangen added maintenance User-facing maintenance tasks needs followup and removed needs followup labels Sep 28, 2022
@ndelangen
Copy link
Member

I can't get the CI to pass. I tried a few times.

I don't get why it's failing either. 😢

@ndelangen ndelangen added build Internal-facing build tooling & test updates and removed maintenance User-facing maintenance tasks labels Oct 4, 2022
@ndelangen ndelangen merged commit be861c8 into storybookjs:next Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Internal-facing build tooling & test updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants