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

fix: handleValueChange.cancel is not a function #625

Conversation

life2015
Copy link
Contributor

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.

Copy link

linux-foundation-easycla bot commented Nov 14, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: life2015 / name: RetroX (94faa92)
  • ✅ login: valscion / name: Vesa Laakso (1443bf0)

@valscion
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

  • revert lodash.debounce change
  • change code to adapt new deboucne dependency

@life2015 life2015 force-pushed the fix/handleValueChange-cancel-not-function branch from 7f41c16 to 2801c02 Compare November 15, 2023 03:02
@life2015
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor

SukkaW commented Nov 15, 2023

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
Copy link
Member

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

Can you also add a changelog entry?

@jodaka
Copy link

jodaka commented Nov 15, 2023

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
Copy link
Member

valscion commented Nov 15, 2023

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
Copy link
Contributor Author

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 life2015 force-pushed the fix/handleValueChange-cancel-not-function branch from 2801c02 to 94faa92 Compare November 16, 2023 03:15
@life2015
Copy link
Contributor Author

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

@valscion
Copy link
Member

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

@valscion valscion merged commit 4c424d5 into webpack-contrib:master Nov 16, 2023
6 checks passed
@valscion
Copy link
Member

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

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

Successfully merging this pull request may close these issues.

4 participants