webpack-contrib / webpack-bundle-analyzer

Webpack plugin and CLI utility that represents bundle content as convenient interactive zoomable treemap
MIT License
12.53k stars 479 forks source link

fix: handleValueChange.cancel is not a function #625

Closed life2015 closed 8 months ago

life2015 commented 8 months ago
image

The error "this.handleValueChange.cancel is not a function" often occurs when I use webpack-bundle-analyzer, leading to a paralysis of page interaction.

Therefore, I have made some defensive fixes in the code here. After the fixes, the generated static pages now perform normally.

linux-foundation-easycla[bot] commented 8 months ago

CLA Signed

The committers listed above are authorized under a signed CLA.

valscion commented 8 months ago

Hi! I'm having trouble understanding what the underlying cause is and thus I'm not yet willing to merge this PR as-is.

Is the this.handleValueChange still something that exists in your problem scenario or is that the thing which is somehow broken? What is it's value when that error about cancel not being a function comes to play?

life2015 commented 8 months ago

Hi! I'm having trouble understanding what the underlying cause is and thus I'm not yet willing to merge this PR as-is.

Is the this.handleValueChange still something that exists in your problem scenario or is that the thing which is somehow broken? What is it's value when that error about cancel not being a function comes to play?

I set a breakpoint here, and then I find cancel function is undefined,

image image

View the implementation of the corresponding obfuscated function,

image

I find out it is the source code of package debounce, it's source code :

image

There is no cancel function in debounce,maybe remove cancel() or replace this debounce package is also a good idea.

I don't know what the history of this part of the code is, so I just made a defensive fix.

life2015 commented 8 months ago

This new debounce package is completely different from the old on lodash.debounce, I thought of two ways to solve the problem:

life2015 commented 8 months ago

Hi! I'm having trouble understanding what the underlying cause is and thus I'm not yet willing to merge this PR as-is.

Is the this.handleValueChange still something that exists in your problem scenario or is that the thing which is somehow broken? What is it's value when that error about cancel not being a function comes to play?

Just change my code and force pushed, revert to lodash.debounce is a better idea.

valscion commented 8 months ago

Hey @SukkaW seems like the lodash package removals in

likely caused this bug.

It looks like to me that the proper fix would be to use the .clear() method instead of .cancel() — it seems to serve the same behavior as before.

SukkaW commented 8 months ago

Hey @SukkaW seems like the lodash package removals in

likely caused this bug.

It looks like to me that the proper fix would be to use the .clear() method instead of .cancel() — it seems to serve the same behavior as before.

Oops. But since the debounce package does provide the clear, IMHO we can just switch to that and stick with debounce. @life2015 What do you think?

valscion commented 8 months ago

Yeah let's use the clear() function coming from the debounce package.

Can you also add a changelog entry?

jodaka commented 8 months ago

And while this isn't merged and released, I guess the only option for the rest of us would be to downgrade to 4.9.1, as this error makes it pretty much impossible to use report page

valscion commented 8 months ago

Yeah I can retag 4.9.1 as the @latest in npm so 4.10.0 isn't installed automatically for people.

EDIT: Did just that now. I'll make sure to update the latest tag in npm to a fixed version once that's out.

life2015 commented 8 months ago

Hey @SukkaW seems like the lodash package removals in

likely caused this bug. It looks like to me that the proper fix would be to use the .clear() method instead of .cancel() — it seems to serve the same behavior as before.

Oops. But since the debounce package does provide the clear, IMHO we can just switch to that and stick with debounce. @life2015 What do you think?

OK, I will use debounce package, and change cancel() to clear()

life2015 commented 8 months ago

Just changed my code and force pushed, and added CHANGELOG. @valscion

valscion commented 8 months ago

The changelog entry is in the wrong place but I'll move that and merge. Thanks!

valscion commented 8 months ago

Released as v4.10.1 and tagged as latest in npm. Thanks for the PR, @life2015! ☺️