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

storybook: disable preserveSymlinks for vite resolve #721

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

emersion
Copy link
Member

@emersion emersion commented Nov 25, 2024

preserveSymlinks is for yarn only, it breaks NPM (keeps an old version of the dependencies around).

To test:

  • npm run watch in ui-manchette and ui-manchette-with-spacetimechart
  • npm run storybook
  • Open the ui-manchette-with-spacetimechart story in the browser
  • Add a console.log() in the Waypoint component

Before this patch, the log would never show up in the console. With this patch, it should live reload and show up.

Works on my machine™ as well as @SharglutDev's.

preserveSymlinks is for yarn only, it breaks NPM.

Signed-off-by: Simon Ser <[email protected]>
@SharglutDev
Copy link
Contributor

Lgtm for me but maybe it's better if other devs test it as well !

Copy link
Contributor

@theocrsb theocrsb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and tested

@clarani
Copy link
Contributor

clarani commented Nov 28, 2024

It does not work on my machine :/

@emersion
Copy link
Member Author

emersion commented Dec 4, 2024

It's not a 100% reliable solution, I've seen it fail on my machine as well. It's just that, before this PR it was never working, and now it's working at least some of the time.

My guess is that there is something wrong with the watched files, and the reason it works sometimes is that style.css gets regenerated and triggers a rebuild/reload as a side effect, but it's racy.

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.

4 participants