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: prevent possible errors when inputs property is not set #8012

Merged
merged 2 commits into from
Oct 24, 2024

Conversation

web-padawan
Copy link
Member

Description

Fixes vaadin/flow-components#6739

Updated checkValidity() method to check that this.inputs and this.value are defined before accessing them.

Type of change

  • Bugfix

const invalidFields = this.inputs.filter((input) => !(input.validate || input.checkValidity).call(input));

if (invalidFields.length || (this.required && !this.value.trim())) {
if (

Choose a reason for hiding this comment

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

Looks like this is the wrong place to fix the problem completely.

The same thing happens as well in other methods, e.g. here:

_shouldRemoveFocus(event) {
      const { relatedTarget } = event;
      return !this.inputs.some((el) => relatedTarget === (el.focusElement || el));
    }

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, thanks. I will check other methods too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated the PR. I wasn't able to reproduce this exact error though as typically the web component can only receive and then lose focus when it is connected to the DOM and by this point of time inputs is set to empty array.

@web-padawan web-padawan changed the title fix: prevent possible errors in custom-field checkValidity fix: prevent possible errors when inputs property is not set Oct 24, 2024
@web-padawan web-padawan force-pushed the fix/custom-field-check-validity branch 2 times, most recently from c968a1a to fc26ed1 Compare October 24, 2024 07:44
@web-padawan web-padawan force-pushed the fix/custom-field-check-validity branch from fc26ed1 to 70770f8 Compare October 24, 2024 07:50
Copy link

sonarcloud bot commented Oct 24, 2024

@web-padawan web-padawan merged commit 82bfd11 into main Oct 24, 2024
9 checks passed
@web-padawan web-padawan deleted the fix/custom-field-check-validity branch October 24, 2024 08:05
@vaadin-bot
Copy link
Collaborator

Hi @web-padawan and @web-padawan, when i performed cherry-pick to this commit to 24.4, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 82bfd11
error: could not apply 82bfd11... fix: prevent possible errors when inputs property is not set (#8012)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

@rappenze
Copy link

@web-padawan Just seen that you added target 24.4 but I never had this problems in 24.4. So maybe backport is not needed as there is all ok?

web-padawan added a commit that referenced this pull request Oct 24, 2024
@lmorocz
Copy link

lmorocz commented Nov 25, 2024

We do have problems with the original change using the latest v24.4.17 platform (v24.4.12 Vaadin Flow) version. In some cases the inputs property is still undefined in the ready phase:

generated-flow-imports-CjbTJID0.js:7519 Uncaught TypeError: Cannot read properties of undefined (reading 'filter')
    at CustomField.checkValidity (generated-flow-impor…CjbTJID0.js:7519:39)
    at CustomField.validate (indexhtml-GaOLT410.js:20753:28)
    at CustomField._requiredChanged (generated-flow-impor…CjbTJID0.js:7535:12)
    at Object.runMethodEffect [as fn] (indexhtml-GaOLT410.js:9666:39)
    at runEffects (indexhtml-GaOLT410.js:9210:16)
    at CustomField._propertiesChanged (indexhtml-GaOLT410.js:10301:7)
    at Dx.s2._propertiesChanged (FlowClient-CfFCB582.js:9090:12)
    at CustomField._flushProperties (indexhtml-GaOLT410.js:8203:16)
    at CustomField.ready (indexhtml-GaOLT410.js:10269:12)
    at CustomField.ready (indexhtml-GaOLT410.js:12080:13)
    at CustomField.ready (indexhtml-GaOLT410.js:15164:13)
    at CustomField.ready (indexhtml-GaOLT410.js:15443:13)
    at CustomField.ready (indexhtml-GaOLT410.js:20709:13)
    at CustomField.ready (indexhtml-GaOLT410.js:20882:11)
    at CustomField.ready (generated-flow-impor…CjbTJID0.js:7465:11)
    at Dx.s2.ready (FlowClient-CfFCB582.js:9095:12)

@web-padawan could you backport this fix to the 24.4 branch as well? 🙏🙇‍♂️

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.

custom field is missing method _setInputs()
5 participants