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

Builder-vite: normalize absolute paths when processing preview annotations #19828

Closed
wants to merge 6 commits into from
Closed

Conversation

Dschungelabenteuer
Copy link
Member

@Dschungelabenteuer Dschungelabenteuer commented Nov 14, 2022

Issue: #19824

What I did

Simply made use of Vite's normalizePath utility function to enfore consistent POSIX paths regardless of the OS.

  • This change obviously does not apply to bare imports
  • Although it does not apply to relative paths, this could be discussed. Applying normalizePath on relative paths should not break anything, and it could possibly address issues if for some reason we encounter a path looking like ../mixed/weird\path\noes

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.

@IanVS
Copy link
Member

IanVS commented Nov 14, 2022

Thanks @Dschungelabenteuer. @joshwooding also put up a PR to fix the same issue (though I think maybe the issue you've mentioned in the description isn't what you intended?)

It sounds like we have a choice to make between slash and normalizePath. Slash is used across much of storybook, so avoiding/removing it here won't actually save any node_modules space. I lean towards using slash to stay consistent with the rest of the SB codebase, unless you have other thoughts @Dschungelabenteuer about why vite's normalizePath might be better.

@Dschungelabenteuer
Copy link
Member Author

(though I think maybe the issue you've mentioned in the description isn't what you intended?)

Oops, made a typo while copying the original issue, fixed it. I should have checked existing PRs before, closing this one in favor of @joshwooding's :)

It sounds like we have a choice to make between slash and normalizePath.

Seems like Vite itself uses slash inside normalizePath

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants