Skip to content

Allow liveReload to be configurable in dev server #3154

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

Merged
merged 1 commit into from
Sep 29, 2021
Merged

Allow liveReload to be configurable in dev server #3154

merged 1 commit into from
Sep 29, 2021

Conversation

t27duck
Copy link
Contributor

@t27duck t27duck commented Sep 2, 2021

By default, liveReload for the webpack_dev_server config is the inverse of the hmr setting. There may be a usecase where one may want to control both values manually.

This allows liveReload to be configurable through webpacker.yml via the live_reload option. If not set, then the default of the inverse of hmr is used.

Copy link

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Looks good except unsure about the contentHash change

Copy link

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Manually tested locally

@PikachuEXE
Copy link

For those who want to test this locally simply update package.json:

    "@rails/webpacker": "https://github.com/t27duck/webpacker.git#live_reload_configurable",

Update config/webpacker.yml if necessary

By default, liveReload for the `webpack_dev_server` config is the inverse of the hmr setting. There may be a usecase where one may want to control both values manually.

This allows liveReload to be configurable through `webpacker.yml` via the `live_reload` option. If not set, then the default of the inverse of hmr is used.

--

As an aside, this also fixes a deprication warning regarding the use of `[hash]` for the output file from the dev server.

```
[DEP_WEBPACK_TEMPLATE_PATH_PLUGIN_REPLACE_PATH_VARIABLES_HASH] DeprecationWarning: [hash] is now [fullhash] (also consider using [chunkhash] or [contenthash], see documentation for details)
```
@t27duck
Copy link
Contributor Author

t27duck commented Sep 22, 2021

Rebased with master. PR #3169 removed the hash part of the config so the "fix the deprication warning" is also removed from this PR.

@PikachuEXE
Copy link

Anything else needs to be done to get this merged?

@guillaumebriday guillaumebriday merged commit 67a94fb into rails:master Sep 29, 2021
@guillaumebriday
Copy link
Member

Sounds good!

@justin808
Copy link
Contributor

@t27duck

There may be a usecase where one may want to control both values manually.

What is the use case that you mention?

@t27duck
Copy link
Contributor Author

t27duck commented Oct 10, 2021

@t27duck

There may be a usecase where one may want to control both values manually.

What is the use case that you mention?

Specifically, @PikachuEXE 🤷‍♂️

@t27duck
Copy link
Contributor Author

t27duck commented Oct 10, 2021

@justin808 more specifically, this comment from the PR when I duct taped webpack-dev-server 4 into webpacker

#3122 (comment)

@PikachuEXE
Copy link

Yup, that comment right there
I have liveReload disabled for ages in current version of webpacker
It's annoying if any small change (sometimes just adding/updating code comment) triggers a reload and lose the state on my dev website page.

@justin808
Copy link
Contributor

@t27duck @PikachuEXE, OK, I get it. If not HMR, then live reload may be on or off.

But what is the point of having liveReload true if HMR is true?

@PikachuEXE
Copy link

It allows "having liveReload true if HMR is true" doesn't mean it's meaningful
But if someone want's to try it I am fine :P (they probably will learn that's meaningless)

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