Skip to content

Conversation

@TomekStaszkiewicz
Copy link
Contributor

See #96

I checked what changed from version 0.4.3 and I noticed that back then the small library deep-extend was used to extend resulting contents.
In version 1.0.0 it was replaced by small util assign, but I believe that someone made a small mistake, and actually the util assignin was meant to be used. After changing to that it seems to be working.

@TomekStaszkiewicz
Copy link
Contributor Author

I was wrong with using the assignin - it is not working as the deep-merge mechanism. It is performing only a shallow merge. I think that using lodash.merge is the best solution for this one

Copy link
Contributor

@joaopslins joaopslins left a comment

Choose a reason for hiding this comment

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

Hi @TomekStaszkiewicz

Thanks for the reproduction and the PR! Would you mind adding some regression tests for this use case of multiple export configurations? Both for webpack 4 and webpack 5? If you aren't able to do that please let us know so we can work on it. Thanks!

Comment on lines 3 to 7
const merge = require('lodash.merge');

const output = { name: '' };

assign(output, { name: 'common' });
merge(output, { name: 'common' });
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this should not be changed, since it is part of the tests. The fixes you had to make in the tests are caused by this, not by changing assign to merge in the _writeOutput method.

I actually find it weird that this change is affecting the current tests, but I'd like for this PR to focus on one matter at a time. If you could please revert this specific change (and the tests), or provide a reason for that change, it would be great.

@joaopslins joaopslins merged commit 0821f04 into django-webpack:master Jun 11, 2021
@andersk
Copy link
Contributor

andersk commented Jul 6, 2021

This has caused unexpected duplication after webpack-dev-server hot updates, due to the following behavior of lodash.merge:

> chunks = { main: ['a', 'b', 'c', 'd', 'e'] } { main: [ 'a', 'b', 'c', 'd', 'e' ] } > merge(chunks, { main: ['foo', 'bar'] }) { main: [ 'foo', 'bar', 'c', 'd', 'e' ] }

Opened #101 to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants