Skip to content

Initialize compilationDependencies in Compilation as undefined - #6885

Merged
TheLarkInn merged 3 commits into
webpack:masterfrom
mohsen1:patch-5
Mar 29, 2018
Merged

TheLarkInn merged 3 commits into
webpack:masterfrom
mohsen1:patch-5

Conversation

@mohsen1

@mohsen1 mohsen1 commented Mar 28, 2018

Copy link
Copy Markdown
Contributor

Related to #6862

@sokra you mentioned this is broken but I'm not sure removing the argument is the right change

Comment thread lib/Compilation.js
}

summarizeDependencies() {
this.fileDependencies = new SortableSet(this.compilationDependencies);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This property exists. It is set in Compiler#newCompilation on line 434.

@webpack-bot

Copy link
Copy Markdown
Contributor

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

@sokra sokra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ooflorent is right this property really exist. You can initialize it to undefined in the Compilation constructor.

@mohsen1 mohsen1 changed the title Remove extra arguments passed to SortableSet initializing fileDependencies Initialize compilationDependencies in Compilation as undefined Mar 28, 2018
@webpack-bot

Copy link
Copy Markdown
Contributor

@mohsen1 Thanks for your update.

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

@sokra Please review the new changes.

@TheLarkInn
TheLarkInn merged commit 8498fc0 into webpack:master Mar 29, 2018
@TheLarkInn

Copy link
Copy Markdown
Member

Thanks

mhegazy added a commit to mhegazy/webpack that referenced this pull request Apr 23, 2018
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.

5 participants