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

feat(Compiler): Add file removal tracking in watch-run #8175

Merged
merged 1 commit into from Oct 24, 2018

Conversation

Projects
None yet
4 participants
@cacheflow
Copy link
Contributor

cacheflow commented Oct 12, 2018

What kind of change does this PR introduce?

  • Feature

Did you add tests for your changes?

  • Yes

Does this PR introduce a breaking change?

  • No

What needs to be documented once your changes are merged?

That watchRun now exposes removed files.

@jsf-clabot

This comment has been minimized.

Copy link

jsf-clabot commented Oct 12, 2018

CLA assistant check
All committers have signed the CLA.

@webpack-bot

This comment has been minimized.

Copy link

webpack-bot commented Oct 12, 2018

For maintainers only:

  • This need to be documented (issue in webpack/webpack.js.org will be filed when merged)

@cacheflow cacheflow changed the title Added tracking of file removal in watch-run Add file removal tracking in watch-run Oct 12, 2018

@cacheflow cacheflow changed the title Add file removal tracking in watch-run feat(Compiler): Add file removal tracking in watch-run Oct 12, 2018

@webpack-bot

This comment has been minimized.

Copy link

webpack-bot commented Oct 12, 2018

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@@ -128,6 +128,7 @@ class Compiler extends Tapable {
/** @type {string|null} */
this.recordsOutputPath = null;
this.records = {};
this.removedFiles = [];

This comment has been minimized.

@sokra

sokra Oct 12, 2018

Member

make it a Set

This comment has been minimized.

@cacheflow

cacheflow Oct 12, 2018

Author Contributor

Done. 👍

@webpack-bot

This comment has been minimized.

Copy link

webpack-bot commented Oct 12, 2018

@cacheflow Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@sokra Please review the new changes.

@@ -57,7 +57,8 @@ class NodeWatchFileSystem {
changes.filter(file => dirs.includes(file)).sort(),
changes.filter(file => missing.includes(file)).sort(),
times,
times
times,
removals

This comment has been minimized.

@sokra

sokra Oct 17, 2018

Member

filter it by files, dirs or missing, sort it, convert it to a Set

This comment has been minimized.

@cacheflow

cacheflow Oct 23, 2018

Author Contributor

Updated @sokra 👍

@@ -57,7 +57,8 @@ class NodeWatchFileSystem {
changes.filter(file => dirs.includes(file)).sort(),
changes.filter(file => missing.includes(file)).sort(),

This comment has been minimized.

@sokra

sokra Oct 17, 2018

Member

For performance reasons, you could convert files, dirs, missing into Sets.

And add a TODO to change the watch(files, dirs, missing signature to Sets in webpack 5.

This comment has been minimized.

@cacheflow

cacheflow Oct 17, 2018

Author Contributor

I'll change them into sets and will also take the TODO to change the signature in Webpack 5.

@cacheflow cacheflow force-pushed the cacheflow:add-tracking-of-removed-files branch from 096ef0c to 993c840 Oct 17, 2018

@cacheflow cacheflow force-pushed the cacheflow:add-tracking-of-removed-files branch 2 times, most recently from dcaee2e to 9db36b8 Oct 17, 2018

@cacheflow cacheflow force-pushed the cacheflow:add-tracking-of-removed-files branch from 9db36b8 to 2cdf04e Oct 18, 2018

@sokra

sokra approved these changes Oct 23, 2018

@sokra sokra merged commit af4cb35 into webpack:master Oct 24, 2018

12 checks passed

codecov/changes/basic No unexpected coverage changes found.
Details
codecov/changes/integration No unexpected coverage changes found.
Details
codecov/changes/unit No unexpected coverage changes found.
Details
codecov/patch/integration 100% of diff hit (target 90%)
Details
codecov/patch/unit Coverage not affected when comparing 1d9f8a3...2cdf04e
Details
codecov/project/basic 100% remains the same compared to 1d9f8a3
Details
codecov/project/integration 91.7% (+0.18%) compared to 1d9f8a3
Details
codecov/project/unit 100% (target 0%)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
security/snyk - package.json (sokra) No manifest changes detected
@sokra

This comment has been minimized.

Copy link
Member

sokra commented Oct 24, 2018

Thanks

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