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

feat(plugin): Add plugin hook to allow other plugins to use the manifest #76

Merged
merged 2 commits into from
Sep 1, 2017

Conversation

D34THWINGS
Copy link
Contributor

Hello there 😃

My team has very specific needs for our build process and we are currently developing a Webpack Plugin that needs the entry and output filenames. We could do this on our own but it would be pointless to duplicate the code that is in the webpack-manifest-plugin. So we make a proposal to add a Webpack plugin hook to your plugin that will allow any plugin to hook itself to yours to pull the manifest.

I used the applyPluginsAsync function because the hook that I registered is post emit so no transformation could be applied and therefore there's no need to use waterfall or series.

I thought about adding also a hook before Json.stringify but I think that there's more probability that webpack-manifest-plugin hooks on other plugins to generate the manifest than other plugins hooking to webpack-manifest-plugin to edit it.

The hook is webpack-manifest-plugin-after-emit with one argument containing the whole manifest.

Does it sounds good to you ?

Thanks for your work !

@codecov-io
Copy link

codecov-io commented Aug 28, 2017

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #76   +/-   ##
=======================================
  Coverage   98.57%   98.57%           
=======================================
  Files           2        2           
  Lines          70       70           
=======================================
  Hits           69       69           
  Misses          1        1
Impacted Files Coverage Δ
lib/plugin.js 98.55% <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 04916cd...62d8ad4. Read the comment docs.

@mastilver
Copy link
Contributor

Yep, I thought we would need something like that one day 👍

Would you mind adding a test please

@D34THWINGS
Copy link
Contributor Author

@mastilver yeah I fogot it, now it's done !

For contributors, would you like to have a basic eslint linting configuration ? It would ensure codestyle is matching with what's already in place (no missing semi, identation, make sure no ES6 feature is used, etc). I can do a pull request if you want

Copy link
Contributor

@mastilver mastilver left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@mastilver
Copy link
Contributor

yep, already started in #67 (let me know what you think there) but I want to get 2.x out of the door first, then think about code style

@PatrickMennen
Copy link

PatrickMennen commented Nov 3, 2017

Since this was merged, is it also possible to publish a new release on NPM? Testing with the locally changed file the emitter works (which is nice!) but I would rather not modify third party packages in my project.

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.

4 participants