Skip to content
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

Check on existing callback #22

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

navelpluisje
Copy link

@navelpluisje navelpluisje commented Mar 28, 2018

Removed callback from html-webpack-plugin-before-html-generation, because it is not returning a callback (anymore).
fixes #19

Removed callback from `html-webpack-plugin-before-html-generation`, because it is not returning a callback anymore.
@tatobari
Copy link

tatobari commented Apr 5, 2018

Should the callback be removed or, to retain backward compatibility, should the callback function be called with the following validation?

if(typeof cb == 'function'){
  cb(null, data);
}

I'm definitely out of my place here cause I'm no programmer, but looks safer to me.

@navelpluisje
Copy link
Author

@tatobari Good point. I did some tests before this change and did not get any callback function returned, that's why I removed it. But for backward compatibility I will change it.

And now let's hope it will be merged

@tatobari
Copy link

tatobari commented Apr 5, 2018

@navelpluisje Awesome man! Let's hope @gabiseabra has time to check your PR.

@navelpluisje navelpluisje changed the title Removed unparsed callback Check on existing callback Apr 5, 2018
@tatobari
Copy link

tatobari commented Apr 5, 2018

@navelpluisje Hate to say this, but line 116 says cd instead of cb. Little code TYPO, I guess.

@navelpluisje
Copy link
Author

@tatobari Changed it. At least it couldn't break anything ;-)

@tatobari
Copy link

tatobari commented Apr 5, 2018

@navelpluisje Job well done, sir.

@ymhr
Copy link

ymhr commented Apr 23, 2018

Is there any chance of this being accepted soon?

@unel
Copy link

unel commented Jun 25, 2018

is here a some life exists?..

@antony
Copy link

antony commented Aug 2, 2018

I'm assuming this project has been abandoned, so I've forked, merged this PR and published the module as @beyonk/google-fonts-webpack-plugin - also on github.

Please test and let me know your findings!

@subins2000
Copy link

Thank you for maintaining this @antony . If you intent on maintaining it, can you please make a separate repo (instead of fork) so we can file issues and work on it there ?

@antony
Copy link

antony commented Jul 7, 2020

A fork is a separate repository. You can file issues there :)

@subins2000
Copy link

@antony I don't see an Issues tab there tho ?

Screenshot_20200707_140724

@antony
Copy link

antony commented Jul 7, 2020

Done.

Btw I don't plan on active maintenance, we use rollup now. But if people open PRs I will review, merge and release.

@subins2000 subins2000 mentioned this pull request Jul 7, 2020
@subins2000
Copy link

@antony Just sent a PR there :)

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.

Webpack 4 and html-webpack-plugin 4 alpha
6 participants