refactor(index): remove `Tapable.apply` calls #121

ooflorent · 2018-01-23T19:50:34Z

Tapable.apply is deprecated in tapable@1.0.0-x.
The plugins should now call apply themselves.

Fixes #136

jsf-clabot · 2018-01-23T19:50:35Z

All committers have signed the CLA.

ooflorent · 2018-01-23T19:51:37Z

src/index.js


  const subCache = `subcache ${__dirname} ${request}`;

+  // TODO use `worker.compiler.hooks.compilation.tap()` instead


This is more subtle than this: the next branch must use webpack@4 before.

michael-ciniawsky

Thx

alexander-akait

@ooflorent this is work on webpack 3 and/or 2? Maybe also next branch?

ooflorent · 2018-01-23T20:02:27Z

@evilebottnawi apply works on v3 and v4 (next branch). Using hooks require v4.

alexander-akait · 2018-01-23T20:33:56Z

@ooflorent thanks for clarify, need accept CLA 😄

ooflorent · 2018-01-23T21:31:49Z

@evilebottnawi done

switz · 2018-03-15T15:00:51Z

@ooflorent looks like the CLA isn't signed yet according to clabot.

alexander-akait · 2018-03-15T15:02:29Z

All fine.
/cc @michael-ciniawsky

ooflorent · 2018-03-21T10:53:38Z

I've updated the PR to target master since webpack 4 compatibility has been added.

shellscape · 2018-04-06T20:58:59Z

@ooflorent this is going to need an upstream rebase to pass CI unfortunately.

renchap · 2018-04-17T14:02:08Z

Any news on this?

WhoAteDaCake · 2018-04-20T14:22:34Z

Is there anything stopping from this being merged?

shellscape · 2018-04-20T14:30:32Z

@WhoAteDaCake yes, as clearly indicated by the conversation and comment made on this PR 14 days ago. please do refrain from comments with questions such as that and the previous "Any news on this?" They don't contribute anything meaningful to the PR or associated issue. The submitter (@ooflorent in this case) will have already been aware of the last comment requesting additional work and they'll come back to it when they have the time available to do so.

ooflorent · 2018-04-23T07:07:51Z

I've rebased the PR.

codecov · 2018-04-23T07:09:49Z

Codecov Report

Merging #121 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #121   +/-   ##
=======================================
  Coverage   66.21%   66.21%           
=======================================
  Files           5        5           
  Lines          74       74           
  Branches       25       25           
=======================================
  Hits           49       49           
  Misses         22       22           
  Partials        3        3

Impacted Files	Coverage Δ
src/index.js	`88.88% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4c00e5...bdbd270. Read the comment docs.

mikegreiling · 2018-05-14T21:31:04Z

@shellscape or @d3viant0ne Could we get a new version tagged with this change, please? This is the only remaining loader/plugin in my webpack config that is still throwing deprecation warnings in webpack v4 and it's driving me nuts 😅

🙏

* Remove Tapable.apply calls * chore: add code comment for clarity, trigger circle

ooflorent commented Jan 23, 2018

View reviewed changes

michael-ciniawsky changed the title ~~Remove Tapable.apply calls~~ refactor(index): remove Tapable.apply calls Jan 23, 2018

michael-ciniawsky added this to the 2.0.0 milestone Jan 23, 2018

michael-ciniawsky added pr_action: Review status: Pending semver: Patch type: Refactor branch: next labels Jan 23, 2018

michael-ciniawsky requested review from alexander-akait, michael-ciniawsky, TrySound and joshwiens January 23, 2018 19:57

michael-ciniawsky approved these changes Jan 23, 2018

View reviewed changes

alexander-akait reviewed Jan 23, 2018

View reviewed changes

michael-ciniawsky removed the branch: next label Feb 27, 2018

alexander-akait approved these changes Mar 15, 2018

View reviewed changes

alexander-akait closed this Mar 15, 2018

alexander-akait reopened this Mar 15, 2018

ooflorent mentioned this pull request Mar 21, 2018

fix: Don't use deperecated Tapable#apply #137

Closed

ooflorent force-pushed the tapable_v1 branch from 8de49e4 to 6a865e6 Compare March 21, 2018 10:52

ooflorent changed the base branch from next to master March 21, 2018 10:52

switz mentioned this pull request Mar 28, 2018

refactor(loader) v2.0.0 #113

Merged

ooflorent and others added 2 commits April 23, 2018 09:06

Remove Tapable.apply calls

86588de

chore: add code comment for clarity, trigger circle

bdbd270

ooflorent force-pushed the tapable_v1 branch from 79a0b4c to bdbd270 Compare April 23, 2018 07:06

shellscape approved these changes Apr 23, 2018

View reviewed changes

shellscape merged commit 04bc4ff into webpack-contrib:master Apr 23, 2018

TheLD6978 pushed a commit to TheLD6978/worker-loader that referenced this pull request Apr 16, 2021

refactor(index): remove Tapable.apply calls (webpack-contrib#121)

d06e6a7

* Remove Tapable.apply calls * chore: add code comment for clarity, trigger circle

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(index): remove `Tapable.apply` calls #121

refactor(index): remove `Tapable.apply` calls #121

ooflorent commented Jan 23, 2018 •

edited

Loading

jsf-clabot commented Jan 23, 2018 •

edited

Loading

ooflorent Jan 23, 2018

michael-ciniawsky left a comment

alexander-akait left a comment

ooflorent commented Jan 23, 2018

alexander-akait commented Jan 23, 2018

ooflorent commented Jan 23, 2018

switz commented Mar 15, 2018

alexander-akait commented Mar 15, 2018

ooflorent commented Mar 21, 2018

shellscape commented Apr 6, 2018 •

edited

Loading

renchap commented Apr 17, 2018

WhoAteDaCake commented Apr 20, 2018

shellscape commented Apr 20, 2018 •

edited

Loading

ooflorent commented Apr 23, 2018

codecov bot commented Apr 23, 2018

mikegreiling commented May 14, 2018


		const subCache = `subcache ${__dirname} ${request}`;

		// TODO use `worker.compiler.hooks.compilation.tap()` instead

refactor(index): remove Tapable.apply calls #121

refactor(index): remove Tapable.apply calls #121

Conversation

ooflorent commented Jan 23, 2018 • edited Loading

jsf-clabot commented Jan 23, 2018 • edited Loading

ooflorent Jan 23, 2018

Choose a reason for hiding this comment

michael-ciniawsky left a comment

Choose a reason for hiding this comment

alexander-akait left a comment

Choose a reason for hiding this comment

ooflorent commented Jan 23, 2018

alexander-akait commented Jan 23, 2018

ooflorent commented Jan 23, 2018

switz commented Mar 15, 2018

alexander-akait commented Mar 15, 2018

ooflorent commented Mar 21, 2018

shellscape commented Apr 6, 2018 • edited Loading

renchap commented Apr 17, 2018

WhoAteDaCake commented Apr 20, 2018

shellscape commented Apr 20, 2018 • edited Loading

ooflorent commented Apr 23, 2018

codecov bot commented Apr 23, 2018

Codecov Report

mikegreiling commented May 14, 2018

refactor(index): remove `Tapable.apply` calls #121

refactor(index): remove `Tapable.apply` calls #121

ooflorent commented Jan 23, 2018 •

edited

Loading

jsf-clabot commented Jan 23, 2018 •

edited

Loading

shellscape commented Apr 6, 2018 •

edited

Loading

shellscape commented Apr 20, 2018 •

edited

Loading