fix: support webpack 5 FileSystemInfo API #190
Conversation
Codecov Report
@@ Coverage Diff @@
## master #190 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 6 6
Lines 102 112 +10
Branches 20 23 +3
=====================================
+ Hits 102 112 +10
Continue to review full report at Codecov.
|
ricardogobbosouza
left a comment
ricardogobbosouza
left a comment
There was a problem hiding this comment.
Hi @vankop
Some considerations
Thanks
| } | ||
|
|
||
| return fileTimestamps; | ||
| } |
There was a problem hiding this comment.
We have a big loss here mapping all the entries.
No need this function
| mapFileTimestampsToOldFormat(compilation.fileSystemInfo._fileTimestamps) | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
No need this function, create a const in apply function
| this.isFirstRun = true; | ||
| } | ||
|
|
||
| apply(compilation, callback) { |
There was a problem hiding this comment.
Create a const here to check if exists fileSystemInfo
const fileTimestamps = compilation.fileTimestamps || compilation.fileSystemInfo.getDeprecatedFileTimestamps()| if (this.isFirstRun) { | ||
| this.isFirstRun = false; | ||
| this.prevTimestamps = compilation.fileTimestamps; | ||
| this.prevTimestamps = getFileTimestamps(compilation); |
There was a problem hiding this comment.
Use new const fileTimestamps
| } | ||
|
|
||
| const dirtyOptions = { ...this.options }; | ||
| const newTimestamps = getFileTimestamps(compilation); |
| const newTimestamps = getFileTimestamps(compilation); | ||
| const glob = dirtyOptions.files.join('|').replace(/\\/g, '/'); | ||
| const changedFiles = this.getChangedFiles(compilation.fileTimestamps, glob); | ||
| const changedFiles = this.getChangedFiles(newTimestamps, glob); |
There was a problem hiding this comment.
Use new const fileTimestamps
| const changedFiles = this.getChangedFiles(newTimestamps, glob); | ||
|
|
||
| this.prevTimestamps = compilation.fileTimestamps; | ||
| this.prevTimestamps = newTimestamps; |
There was a problem hiding this comment.
Use new const fileTimestamps
|
We need to add tests |
| return ( | ||
| compilation.fileTimestamps || | ||
| // eslint-disable-next-line no-underscore-dangle | ||
| mapFileTimestampsToOldFormat(compilation.fileSystemInfo._fileTimestamps) |
There was a problem hiding this comment.
_fileTimestamps is private. Don't do that! That's not how FileSystemInfo works. fileSystemInfo._fileTimestamps is a cache, while in webpack 4 compilation.fileTimestamps was a complete list.
Use the public functions to access timestamps of files.
There was a problem hiding this comment.
Just use getDeprecatedFileTimestamps from FileSystemInfo
There was a problem hiding this comment.
@sokra could we have a method getFileTimestamps so as not to depend on a getDeprecatedFileTimestamps function that will be depreciated?
getFileTimestamps() {
return this._fileTimestamps;
}
|
Thanks for all your replies!
I found out that it is cache (all timestamps that was created via
@ricardogobbosouza Unfortunately
So I can upgrade dev dependency to |
|
Lets wait for webpack/webpack#9679 |
|
@vankop, seems like the PR was merged and should be included with the next beta (beta.7) when released. |
|
beta.7 is out and does include the PR |
|
/cc @vankop @ricardogobbosouza what is status? |
|
@evilebottnawi sorry I did not have time, hope on the nearest weekend I can try to migrate on beta.7 . |
|
@vankop @evilebottnawi |
7 participants
This PR contains a:
Motivation / Use-Case
#187
Breaking Changes
no
Additional Info
no