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

Handle asset source being a buffer, not a string #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

praxxis
Copy link

@praxxis praxxis commented Mar 29, 2018

I am still trying to track down why this happens, but when doing a production build source() returns a Buffer instead of a string. It seems to be fine in development, so my intuition is that it's minification related, but I haven't been able to isolate exactly which option. Changing mode, disabling the Uglify plugin - nothing seems to alter how this works in a production build.

Example in production build (output of typeof compilation.assets[manifestPath].source()):

type of source: object

vs dev

type of source: string

I am still trying to track down _why_ this happens, but when doing a production build `source()` returns a `Buffer` instead of a string. It seems to be fine in development, so my intuition is that it's minification related, but I haven't been able to isolate exactly which option. Changing `mode`, disabling the Uglify plugin - nothing seems to alter how this works in a production build.
@almothafar
Copy link
Owner

Thanks for this PR, for webpack 3 I see this is working correctly, so I'm not sure what this should fix, also in most webpack plugins that I saw and that is working with webpack 4, there are no problems that need this change, may you please provide me with any source that I can see the problem and the resolution? or at least a similar issue that fixed in this way on another plugin.

@praxxis
Copy link
Author

praxxis commented Apr 2, 2018

I've tracked this down to imagemin-webpack-plugin. I have an example repo up at https://github.com/praxxis/manifest-plugin-bug that shows the problem. From my debugging it seems that including the imagemin plugin turns source assets into RawSource instances, instead of CachedSource or SourceMapSource. RawSource.source() doesn't seem to produce the same output, hence .replace() not being valid.

@praxxis
Copy link
Author

praxxis commented Apr 2, 2018

webpack/webpack-sources#26 looks like a similar bug, and it points to Webpack's implementation being inconsistent

@praxxis
Copy link
Author

praxxis commented Apr 2, 2018

a workaround for anyone else hitting this: change the test option of imagemin to only include image types

@almothafar
Copy link
Owner

I think extra .toString() does not hurt even if no issues here, but still I don't see it is necessary to merge this if it is another plugin issues, as you said it is imagemin and there is a workaround for it.

Also, Object.toString() will get the output like "[object Object]", isn't right?

Anyways, I really not have that much experience of webpack plugins development, I will look into your repo and try to figure out what the problem.

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.

2 participants