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

Windows: Path normalisation for changedFiles/base path (TurboSnap) #881

Open
markusnissl opened this issue Dec 27, 2023 · 0 comments
Open
Labels
bug Classification: Something isn't working CLI needs triage Tracking: Issue needs confirmation Support Priority

Comments

@markusnissl
Copy link

markusnissl commented Dec 27, 2023

Bug report

I faced the issue that the TurboSnap functionality is not detecting changed stories on windows while it works on Mac. I traced the issue and detected following bugs:

  • When using the "trace" utility the path of changedFiles are not converted to posix, so providing paths with backslashes will not detect changed stories.
  • When using the "upload" utility with --only-changed the base dir is different between Mac and Windows, causing issue on Windows.

You can try it out by running node inside your repository and execute

Mac:

path = require('path');
path.posix.relative("/<your path to repository>", '') // prints ''

Windows:

path = require('path');
path.posix.relative("C:/<your path to repository>", '') // prints '../../../' << ../ depending on depth of repository

but without the hard drive letter it works fine

path = require('path');
path.posix.relative("<your path to repository>", '') // prints '' 

Tracing this to the root issue:

  • The base dir is defined in line 78 (current main) in getDependentStoryFiles.ts
    const baseDir = storybookBaseDir ? posix(storybookBaseDir) : path.posix.relative(rootPath, '');
    and this base dir behaves differently between Windows and Mac. The reason for this is that rootPath on Windows contains the hard drive letter (causing exactly the wrong relative path) as this path is not treated as an absolute path in posix due to not starting with '/'.
  • The root cause for having the hard driver letter included is related to line 77, a call to getRepositoryRoot which assumes to always have returned an absolute "posix" path, however the command git rev-parse --show-toplevel returns a posix path but with hard disk drive letter, which causes the issue.

Not sure whether there is other code as well that is affected by a similar issue.

Workaround:
Provide a relative --storybook-base-dir path (e.g., just "./") so that it works also on Windows

@markusnissl markusnissl added bug Classification: Something isn't working needs triage Tracking: Issue needs confirmation labels Dec 27, 2023
@linear linear bot added the Empathy label Jan 2, 2024
@linear linear bot unassigned weeksling Feb 6, 2024
@thafryer thafryer assigned thafryer and unassigned thafryer Feb 23, 2024
@thafryer thafryer removed the Empathy label Mar 26, 2024
@linear linear bot added the Paper Cut label Jun 7, 2024
@linear linear bot added CLI and removed Paper Cut labels Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Classification: Something isn't working CLI needs triage Tracking: Issue needs confirmation Support Priority
Projects
None yet
Development

No branches or pull requests

3 participants