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

Vite HMR config does not override Ladle default config #557

Open
park-g opened this issue Apr 16, 2024 · 2 comments · May be fixed by #558
Open

Vite HMR config does not override Ladle default config #557

park-g opened this issue Apr 16, 2024 · 2 comments · May be fixed by #558
Labels
needs triage needs to be reviewed

Comments

@park-g
Copy link

park-g commented Apr 16, 2024

Describe the bug

When trying to use vite.config.mjs to override the default HMR settings (server.hmr) the settings in vite.config.mjs are never applied.

Reproduction

I'm trying to run ladle within a docker container. The problem I am having is that the WS connection to my docker container fails and to fix this I need to override the HMR host. Essentially:

  • Since version 17, nodejs favours IPV6.
  • When using localhost as the HMR host (forced by ladle) starts server on IPV6.
  • I am currently unable to use IPV6 with my docker setup.

To reproduce this issue simply attempt to change server.hmr.host or server.hmr.port in your .ladle/vite.config.mjs and it will not be applied.

I think I have found the issue within the codebase:

  • Here a hardcoded default HMR config can be found.
  • This hardcoded config gets merged into the user config here and overwrites any user HMR settings. This issue says that these settings should work so I am assuming something has accidentally broken this section of the config since then.

Potential Fix

I'm not 100% certain, but from a quick glance I think a fix would be adding HMR settings to the ladle config, and then merging them in here as we do with the other settings in the ladle config.

@park-g park-g added the needs triage needs to be reviewed label Apr 16, 2024
@park-g park-g linked a pull request Apr 16, 2024 that will close this issue
@park-g
Copy link
Author

park-g commented Apr 16, 2024

This PR is not ready to be merged, I don't have time to get the project set up locally right now to test things fully, but this should be a good example of a fix for the problem: #558

@park-g
Copy link
Author

park-g commented May 1, 2024

@tajo any chance you could help me get a fix together and merged for this? I've already attempted a PR here #558, any suggestions or assistance would be greatly appreciated. 🙇🏻

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

Successfully merging a pull request may close this issue.

1 participant