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: pathing of managerEntries containing long paths with hidden folders #20550

Merged
merged 2 commits into from
Jan 10, 2023

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Jan 9, 2023

Issue: #20427
Issue: #20513

What I did

  • Ensured that when writing files to the cache wrapping managerEntries, to flatten the directory structure, removing special characters such as @ and . which might have meaning in the file-system.
  • Strip away some redundant path info
  • Ensure each entry is unique by adding the index into the path

@ndelangen ndelangen self-assigned this Jan 9, 2023
@ndelangen ndelangen added the bug label Jan 9, 2023
@ndelangen ndelangen changed the title change the base locations of the managerEntries files in the cache dir, removing directory depth, removing special characters Fix: pathing of managerEntries containing long paths with hidden folders inside. Jan 9, 2023
@ndelangen ndelangen changed the title Fix: pathing of managerEntries containing long paths with hidden folders inside. Fix: pathing of managerEntries containing long paths with hidden folders Jan 9, 2023
@ndelangen ndelangen requested a review from IanVS January 9, 2023 19:14
@ndelangen
Copy link
Member Author

@Jarvis1010 @fmal If you'd care to take a look, perhaps review? I think this will resolve both issues.

@Jarvis1010
Copy link
Contributor

Looks like a great fix from what I can see

@fmal
Copy link

fmal commented Jan 10, 2023

@ndelangen i have unplugged @storybook/builder-manager and applied your changes.

These are the sanitized paths after changes:
Screenshot 2023-01-10 at 10 20 46

And index.html from manager bundle:

Screenshot 2023-01-10 at 10 22 46

So it indeed resolved the issue! Now I can successfully serve the production build.

Thanks so much 👍

@fmal
Copy link

fmal commented Jan 10, 2023

@IanVS I can confirm this resolves the issue, so would appreciate merge and pre-release 😃 Thanks guys 👍

@ndelangen
Copy link
Member Author

@shilman self merging, can you do a release, please?

@ndelangen ndelangen merged commit a43a3e7 into next Jan 10, 2023
@ndelangen ndelangen deleted the norbert/fix-20427-20513 branch January 10, 2023 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants