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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: configure watching options #486

Open
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@Kocal
Copy link
Contributor

Kocal commented Jan 7, 2019

This PR adds support for properly configuring watching options.
References: #35, #126

Before, we had to do this:

const config = Encore.getWebpackConfig();

if (config.devServer) {
  config.devServer.watchOptions.poll = 250;
}

Now, we can do this:

Encore.configureWatchOptions(watchOptions => {
    watchOptions.poll = 250;
});

Also, I think it would be better to rename it to configureWatchOptions(), but I don't know how encore apply watching options to webpack when running dev or prod environment with encore [prod|dev] --watch. 馃

Thanks!


EDIT: Ah, I found it!
selection_052

I will see how can I modify watchOptions in dev/prod modes.

@Kocal Kocal force-pushed the Kocal:feat/configure-watch-options branch from 6751e5e to e6eb6e1 Jan 7, 2019

@Lyrkan

This comment has been minimized.

Copy link
Collaborator

Lyrkan commented Jan 7, 2019

Hi @Kocal,

Encore.configureDevServerWatchOptions() feels a bit long to me, configureWatchOptions() would probably be better imo.

That being said... I'm wondering if we shouldn't have a configureDevServer instead? Is watchOptions really that special to justify giving it its own method?

@Kocal

This comment has been minimized.

Copy link
Contributor

Kocal commented Jan 7, 2019

Watch mode can be used without a dev-server, so I think it's better to have a .configureWatchOptions() methods that applies both for webpack and webpack-dev-server.

@Kocal Kocal changed the title feat: configure devServer watching options feat: configure watching options Jan 7, 2019

Show resolved Hide resolved lib/config-generator.js Outdated
@Kocal

This comment has been minimized.

Copy link
Contributor

Kocal commented Jan 7, 2019

What should I do with failing tests? It's working fine on local 馃槙
capture d ecran de 2019-01-07 20-22-01

@Lyrkan

This comment has been minimized.

Copy link
Collaborator

Lyrkan commented Jan 7, 2019

Don't worry about the failing tests, they are not related to your PR :)

You don't have them locally because they are in a Travis job that tests the highest possible version of the dependencies (it removes the lock file before downloading them) and sometimes Webpack changes something that causes the hardcoded hashes from our functional tests to become outdated.

@Kocal

This comment has been minimized.

Copy link
Contributor

Kocal commented Jan 7, 2019

Oh okay, thanks for the explanation 馃憤

@Kocal

This comment has been minimized.

Copy link
Contributor

Kocal commented Jan 21, 2019

Who is ready for a new review? 馃樅

@Lyrkan

Lyrkan approved these changes Jan 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment