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

v4.0.0-beta.0: static.watch is enabled by default causing liveReload to always reload page on change #2893

Closed
1 of 2 tasks
akphi opened this issue Dec 1, 2020 · 8 comments · Fixed by #2897
Closed
1 of 2 tasks

Comments

@akphi
Copy link

akphi commented Dec 1, 2020

  • Operating System: MacOS 11.0.1 (Big Sur)
  • Node Version: 14.13.0
  • NPM Version: 6.14.8
  • Yarn version: 1.22.5
  • webpack Version: 5.9.0
  • webpack-dev-server Version: 4.0.0-beta.0
  • Browser: Chrome Version 87.0.4280.67
  • This is a bug
  • This is a modification request

Expected Behavior

A bit of background first. I use react-refresh in my project. In version 3, when I make a change in my JSX file, the page does not reload as HMR takes place. In version 4, the page proceeds with a full reload, causing me to lose all app states.

The doc for v3 mentions that watchBaseContent is disabled by default and thus, this effectively means liveReload=true does not take any effect.

Actual Behavior

When I explicitly set static.watch=true in v4, the issue is resolved. I took a look at the recent change and it seems that in v3, we did this

if (this.options.watchContentBase) {
  runnableFeatures.push('watchContentBase');
}

whereas in v4, we did this

// Server.js
if (this.options.static) {
  runnableFeatures.push('static');
}

if (this.options.static) {
  runnableFeatures.push('staticServeIndex', 'staticWatch');
}

// normalizeOptions.js
const watchOptionsConfig = configArr.find(
    (config) => config.watch !== false && config.watchOptions
  );
  const watchOptions = watchOptionsConfig
    ? watchOptionsConfig.watchOptions
    : {};

  const defaultOptionsForStatic = {
    directory: process.cwd(),
    staticOptions: {},
    publicPath: ['/'],
    serveIndex: { icons: true },
    // Respect options from compiler watchOptions
    watch: watchOptions,
  };

@hiroppy Maybe I misunderstood here, but this will try to get the watchOptions config from webpack and thus almost guarantee to be truthy if we're in development mode. Should we rethink this behavior or clarify it better in the doc/changelog since not everyone uses webpack-dev-server use it for serving static files?

How to Reproduce The Issue

I have a reproducible repo, please take a look at this PR. After upgrading to webpack-dev-server@4.0.0-beta.0. If you run the app using yarn start and try to hit the + button a few time to increase the age, then you try to make some trivial change in App.tsx the page is reloaded, and the age is reset to 10.

On the other hand, if we add the following to devServer section in webpack.config.js, things will behave as expected:

...
  devServer: {
      static: {
        watch: false,
      },
  ...
  }
@alexander-akait
Copy link
Member

hm, I think we should disable static by default

@akphi
Copy link
Author

akphi commented Dec 2, 2020

@alexander-akait that would be great, can we include this in the next release? If this is not too complex, I can give it a shot.

@alexander-akait
Copy link
Member

alexander-akait commented Dec 2, 2020

I think we can do it more in 0CJS way, let's watch by default path.resolve(process.cwd(), 'static') instead process.cwd(), this approach is used by most developers, so if you will not have static directory nothing will be broken

@akphi
Copy link
Author

akphi commented Dec 2, 2020

That makes sense since it's what we have as the example for static in the release changelog

@alexander-akait
Copy link
Member

Yes, I will change it in the next release

@akphi
Copy link
Author

akphi commented Dec 2, 2020

@alexander-akait Thanks!

@jueinin
Copy link

jueinin commented Dec 2, 2020

when watch property is set to true, my project start time has lasted up to 3 minutes. compared with false property of watch, it's 22 seconds...

@alexander-akait
Copy link
Member

@jueinin I think this #2893 (comment) solve your problem too

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 a pull request may close this issue.

3 participants