Skip to content

Conversation

marisveide
Copy link

Fixes the issue: #2000

@gauravtiwari
Copy link
Member

Thanks @marisveide

The reason for not allowing config options in dev server runner is because of sync problem in options like host and port etc since they are used on both Ruby and JS side and it's hard to keep them in sync if we have too many sources to set them up (plus it might be confusing for some people).

At the moment, Webpacker supports .yml file and environment variables

Perhaps, we can allow all other options except certain like host and port. This may need some testing.

@marisveide
Copy link
Author

@gauravtiwari - so are you saying this Pull Request will not get rejected?

This change really helped us set up our development environments properly. And looks like it helped others too already who need to pass the file names from the command line...

@nicklozon
Copy link

nicklozon commented Apr 22, 2019

@gauravtiwari using webpacker.yml and environment variables is okay, but it does make the configuration less flexible because @marisveide will need to be copying his key and cert into the yaml file whenever it changes.

An issue I ran into is the webpack-dev-server https configuration isn't a boolean, but webpacker's reverse proxy won't use https unless that configuration (or the environment variable) is explicitly set to true, and if it is set to true then the key and cert files will never make it to the webpack-dev-server configuration.

Here is my comment about it: #2000 (comment)

Edit: Let us know if the work-around I made is acceptable - I can write a test and open PR. It's not perfect, but it does avoid having too many points of configuration and allows for https for the proxy while using certs/keys into the webpacker.yml file for webpack-dev-server configuration.

@dj0nes
Copy link

dj0nes commented Jun 26, 2019

Ran into this issue in development.

@gauravtiwari - If we filter out host and port from the argument list, would we be closer to getting this feature merged?

@marisveide
Copy link
Author

Yeah, it would be great to merge this... otherwise we are sitting on our fork, and would be glad to not have to maintain the forked version just because we need to pass the SSL certificate params.

@marisveide
Copy link
Author

Hi!

What do I have to do to get this PR merged?

Thanks!

Maris

@jipiboily
Copy link

Is there something we can do to help that get merged?

I'm currently using this workaround: #2000 (comment)

@gauravtiwari
Copy link
Member

@marisveide and everyone, apologies for long silence 🙏

One solution:

If you can filter out the options supported in webpacker.yml inside dev server then I am happy to review. Perhaps, if one supplies the options already supported in webpacker.yml, we can display a friendly warning i.e. "Please set the option in webpacker.yml instead"

The reason to do this is because, we use some options on both node and ruby side (host, port etc.) and if there are multiple ways to declare, it will cause confusion and issues.


I know this still doesn't solve the whole problem, for example colleagues working together want to change these options on runtime without committing. Maybe let's try to figure out a better solution, together.

Simply, allowing options to dev server would cause other problems.

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.

6 participants