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(stubs): Pass stubbed function to function props #2586

Conversation

markrian
Copy link

@markrian markrian commented Jan 8, 2025

The original implementation, added in
22c7698, passed a string value to a function prop. This instead passes an actual function, which stringifies to the same string.

The problem with the previous approach is that Vue started warning about strings passed to function props in
vuejs/core@7ccd453. As a result, this produced excessive noise in the logs that the user can do nothing about.

Copy link

netlify bot commented Jan 8, 2025

Deploy Preview for vue-test-utils-docs ready!

Name Link
🔨 Latest commit 07a493d
🔍 Latest deploy log https://app.netlify.com/sites/vue-test-utils-docs/deploys/677e96a82cdd340008a7d19a
😎 Deploy Preview https://deploy-preview-2586--vue-test-utils-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@markrian
Copy link
Author

markrian commented Jan 8, 2025

@xanf Would you take a look at this please, given you wrote 22c7698?

For context, see https://gitlab.com/gitlab-org/gitlab-foss/-/jobs/8790444271/viewer, showing the log exceeding the size limit due to these messages.

The original implementation, added in
22c7698, passed a string value to a
function prop. This instead passes an actual function, which stringifies
to the same string.

The problem with the previous approach is that Vue started warning about
strings passed to function props in
vuejs/core@7ccd453.
As a result, this produced excessive noise in the logs that the user
can do nothing about.
@markrian markrian force-pushed the fix-stubbed-function-prop-as-string-warnings branch from 573cea4 to 07a493d Compare January 8, 2025 15:15
@cexbrayat
Copy link
Member

Hi @markrian

Thank you for your contribution.
IIRC we already fixed this in #2413 and released it in v2.4.6

If you are still experimenting this issue with v2.4.6, can you add a unit test to your PR that reproduces the issue and demonstrates your fix please?

@markrian
Copy link
Author

markrian commented Jan 8, 2025

@cexbrayat Aha, I didn't realise. As it happens we're on v2.3.1, but we're also monkey patching createStubs (long story), which recreates the issue that #2413 fixed. So our own monkey patching is to blame.

In any case:

We can "trick" the warning introduced in Vue v3.4.22 (See vuejs/core@7ccd453) by using an array of strings instead of a string (as the new check allows functions and arrays, but not strings).

It seems to me the solution here of passing an actual function with a custom toString method is a bit more robust, since if Vue changes how it checks the value to something like isFunction(value) || (isArray(value) && value.every(isFunction)) then the current approach would cause the warnings to appear again.

WDYT? I'm happy to close this PR anyway if you'd rather keep it as-is.

@cexbrayat
Copy link
Member

👍 I think we're fine for now then, lets' close this.
Thank you anyway!

@cexbrayat cexbrayat closed this Jan 8, 2025
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